Use LLVM verifier in lowering (#5733)

Suggested by zygoloid while looking at #5678 

```
CHECK failure at toolchain/lower/context.cpp:62: !llvm::verifyModule(*llvm_module_, &errs): Verifier errors: Instruction does not dominate all uses!
  %.loc17_46.1.temp = alloca { i1, i32, i32 }, align 8, !dbg !13
  %tuple.elem0.loc17_46.2.tuple.elem = getelementptr inbounds nuw { i1, i32, i32 }, ptr %.loc17_46.1.temp, i32 0, i32 0, !dbg !13
Instruction does not dominate all uses!
  %.loc17_46.1.temp = alloca { i1, i32, i32 }, align 8, !dbg !13
  %tuple.elem1.loc17_46.2.tuple.elem = getelementptr inbounds nuw { i1, i32, i32 }, ptr %.loc17_46.1.temp, i32 0, i32 1, !dbg !13
Instruction does not dominate all uses!
  %.loc17_46.1.temp = alloca { i1, i32, i32 }, align 8, !dbg !13
  %tuple.elem2.loc17_46.2.tuple.elem = getelementptr inbounds nuw { i1, i32, i32 }, ptr %.loc17_46.1.temp, i32 0, i32 2, !dbg !13
```

Adds a `--llvm-verifier` flag to be able to turn this off easily,
particularly for debugging the LLVM IR.

The call workaround is due to a verifier requirement `inlinable function
call in a function with debug info must have a !dbg location`. It
specifically comes up for the `++x` case, with `%1 = call i32
@"_CConvert.8b3d5d6a6c17be04:ImplicitAs.Core.b88d1103f417c6d4"(i32
%other)`. I think #5397 is in the direction of a fix for that, but #5397
was set aside because it puts the debug info in too many places.
Instead, address this by adding a stub location for calls that don't
have a good location. I'm deliberately putting this next to the TODO so
that it's easier to understand the association.

---------

Co-authored-by: Dana Jansens <danakj@orodu.net>

toolchain/driver/compile_subcommand.cpp

@@ -294,6 +294,17 @@ Whether to emit DWARF debug information.
         arg_b.Default(true);
         arg_b.Set(&include_debug_info);
       });
+  b.AddFlag(
+      {
+          .name = "verify-llvm-ir",
+          .help = R"""(
+Whether to run the LLVM verifier on modules.
+)""",
+      },
+      [&](auto& arg_b) {
+        arg_b.Default(true);
+        arg_b.Set(&run_llvm_verifier);
+      });
 }
 
 static constexpr CommandLine::CommandInfo SubcommandInfo = {
@@ -712,8 +723,10 @@ auto CompilationUnit::RunLower() -> void {
     llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> subtrees =
         cache_->tree_and_subtrees_getters();
     module_ = Lower::LowerToLLVM(
-        *llvm_context_, driver_env_->fs, options_->include_debug_info, subtrees,
-        input_filename_, *sem_ir_, &inst_namer, vlog_stream_);
+        *llvm_context_, driver_env_->fs,
+        options_->run_llvm_verifier ? driver_env_->error_stream : nullptr,
+        options_->include_debug_info, subtrees, input_filename_, *sem_ir_,
+        &inst_namer, vlog_stream_);
   });
   if (vlog_stream_) {
     CARBON_VLOG("*** llvm::Module ***\n");

toolchain/driver/compile_subcommand.h

@@ -65,6 +65,7 @@ struct CompileOptions {
   bool builtin_sem_ir = false;
   bool prelude_import = false;
   bool include_debug_info = true;
+  bool run_llvm_verifier = true;
 
   llvm::SmallVector<llvm::StringRef> exclude_dump_file_prefixes;
 };

toolchain/lower/context.cpp

@@ -6,7 +6,9 @@
 
 #include "common/check.h"
 #include "common/growing_range.h"
+#include "common/raw_string_ostream.h"
 #include "common/vlog.h"
+#include "llvm/IR/Verifier.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include "toolchain/lower/file_context.h"
 #include "toolchain/sem_ir/inst_namer.h"
@@ -49,11 +51,17 @@ auto Context::LowerPendingDefinitions() -> void {
   }
 }
 
-auto Context::Finalize() && -> std::unique_ptr<llvm::Module> {
+auto Context::Finalize(
+    llvm::raw_ostream*
+        llvm_verifier_stream) && -> std::unique_ptr<llvm::Module> {
   LowerPendingDefinitions();
 
   file_contexts_.ForEach(
       [](auto, auto& file_context) { file_context->Finalize(); });
+
+  if (llvm_verifier_stream) {
+    CARBON_CHECK(!llvm::verifyModule(*llvm_module_, llvm_verifier_stream));
+  }
   return std::move(llvm_module_);
 }
 

toolchain/lower/context.h

@@ -62,8 +62,10 @@ class Context {
   }
 
   // Finishes lowering and takes ownership of the LLVM module. The context
-  // cannot be used further after calling this.
-  auto Finalize() && -> std::unique_ptr<llvm::Module>;
+  // cannot be used further after calling this. If `llvm_verifier_stream` is
+  // non-null, verifies the LLVM module before returning it.
+  auto Finalize(llvm::raw_ostream*
+                    llvm_verifier_stream) && -> std::unique_ptr<llvm::Module>;
 
   // Returns location information for use with DebugInfo.
   auto GetLocForDI(SemIR::AbsoluteNodeId abs_node_id) -> LocForDI;

toolchain/lower/function_context.cpp

@@ -255,6 +255,12 @@ auto FunctionContext::GetDebugLoc(SemIR::InstId inst_id) -> llvm::DebugLoc {
     // duplicated from the original signature.
     //
     // TODO: Handle this case better.
+    if (sem_ir().insts().Is<SemIR::Call>(inst_id)) {
+      // Return a stub location for calls, because they may be inlineable (an
+      // LLVM verifier issue).
+      return llvm::DILocation::get(builder_.getContext(), -1, -1,
+                                   di_subprogram_);
+    }
     return llvm::DebugLoc();
   }
   return llvm::DILocation::get(builder_.getContext(), loc.line_number,

toolchain/lower/lower.cpp

@@ -14,7 +14,8 @@ namespace Carbon::Lower {
 
 auto LowerToLLVM(
     llvm::LLVMContext& llvm_context,
-    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs, bool want_debug_info,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
+    llvm::raw_ostream* llvm_verifier_stream, bool want_debug_info,
     llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters,
     llvm::StringRef module_name, const SemIR::File& sem_ir,
     const SemIR::InstNamer* inst_namer, llvm::raw_ostream* vlog_stream)
@@ -22,7 +23,7 @@ auto LowerToLLVM(
   Context context(llvm_context, std::move(fs), want_debug_info,
                   tree_and_subtrees_getters, module_name, vlog_stream);
   context.GetFileContext(&sem_ir, inst_namer).LowerDefinitions();
-  return std::move(context).Finalize();
+  return std::move(context).Finalize(llvm_verifier_stream);
 }
 
 }  // namespace Carbon::Lower

toolchain/lower/lower.h

@@ -15,9 +15,13 @@
 namespace Carbon::Lower {
 
 // Lowers SemIR to LLVM IR.
+//
+// `llvm_verifier_stream` should be non-null when verification is desired.
+// TODO: Switch to a struct for parameters.
 auto LowerToLLVM(
     llvm::LLVMContext& llvm_context,
-    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs, bool want_debug_info,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
+    llvm::raw_ostream* llvm_verifier_stream, bool want_debug_info,
     llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters,
     llvm::StringRef module_name, const SemIR::File& sem_ir,
     const SemIR::InstNamer* inst_namer, llvm::raw_ostream* vlog_stream)

toolchain/lower/testdata/impl/import_thunk.carbon

@@ -209,7 +209,7 @@ fn Test(a: A) -> C {
 // CHECK:STDOUT:   %.loc9_19.2.temp = alloca { i32 }, align 8, !dbg !11
 // CHECK:STDOUT:   call void @llvm.lifetime.start.p0(i64 4, ptr %.loc9_19.1.temp), !dbg !11
 // CHECK:STDOUT:   call void @llvm.lifetime.start.p0(i64 4, ptr %.2.temp)
-// CHECK:STDOUT:   call void @"_CConvert.A.Main:ImplicitAs.Core"(ptr %.2.temp, ptr %a)
+// CHECK:STDOUT:   call void @"_CConvert.A.Main:ImplicitAs.Core"(ptr %.2.temp, ptr %a), !dbg !12
 // CHECK:STDOUT:   call void @"_CF.X.Main:I.Main"(ptr %.loc9_19.1.temp, ptr %.2.temp), !dbg !11
 // CHECK:STDOUT:   call void @llvm.lifetime.start.p0(i64 4, ptr %.loc9_19.2.temp), !dbg !11
 // CHECK:STDOUT:   call void @"_CConvert.B.Main:ImplicitAs.Core"(ptr %.loc9_19.2.temp, ptr %.loc9_19.1.temp), !dbg !11
@@ -243,6 +243,7 @@ fn Test(a: A) -> C {
 // CHECK:STDOUT: !9 = !DILocation(line: 9, column: 21, scope: !4)
 // CHECK:STDOUT: !10 = distinct !DISubprogram(name: "F", linkageName: "_CF:thunk.X.Main:I.Main", scope: null, file: !3, line: 9, type: !5, spFlags: DISPFlagDefinition, unit: !2)
 // CHECK:STDOUT: !11 = !DILocation(line: 9, column: 3, scope: !10)
+// CHECK:STDOUT: !12 = !DILocation(line: 4294967295, scope: !10)
 // CHECK:STDOUT: ; ModuleID = 'call_thunk_for_imported_interface.carbon'
 // CHECK:STDOUT: source_filename = "call_thunk_for_imported_interface.carbon"
 // CHECK:STDOUT:

toolchain/lower/testdata/operators/increment.carbon

@@ -0,0 +1,78 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/full.carbon
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/lower/testdata/operators/increment.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lower/testdata/operators/increment.carbon
+
+// --- increment.carbon
+
+library "[[@TEST_NAME]]";
+
+fn IncrSigned() {
+  var from: i32 = 0;
+  ++from;
+}
+
+// CHECK:STDOUT: ; ModuleID = 'increment.carbon'
+// CHECK:STDOUT: source_filename = "increment.carbon"
+// CHECK:STDOUT:
+// CHECK:STDOUT: define void @_CIncrSigned.Main() !dbg !4 {
+// CHECK:STDOUT: entry:
+// CHECK:STDOUT:   %from.var = alloca i32, align 4, !dbg !7
+// CHECK:STDOUT:   call void @llvm.lifetime.start.p0(i64 4, ptr %from.var), !dbg !7
+// CHECK:STDOUT:   store i32 0, ptr %from.var, align 4, !dbg !7
+// CHECK:STDOUT:   call void @"_COp.Int.Core:Inc.Core.be1e879c1ad406d8"(ptr %from.var), !dbg !8
+// CHECK:STDOUT:   ret void, !dbg !9
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: ; Function Attrs: nocallback nofree nosync nounwind willreturn memory(argmem: readwrite)
+// CHECK:STDOUT: declare void @llvm.lifetime.start.p0(i64 immarg, ptr captures(none)) #0
+// CHECK:STDOUT:
+// CHECK:STDOUT: define linkonce_odr void @"_COp.Int.Core:Inc.Core.be1e879c1ad406d8"(ptr %self) !dbg !10 {
+// CHECK:STDOUT:   call void @"_COp:thunk.Int.Core:AddAssignWith.Core.25a9a5e901f5b032"(ptr %self, i32 1), !dbg !12
+// CHECK:STDOUT:   ret void, !dbg !13
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: define linkonce_odr void @"_COp:thunk.Int.Core:AddAssignWith.Core.25a9a5e901f5b032"(ptr %self, i32 %other) !dbg !14 {
+// CHECK:STDOUT:   %1 = call i32 @"_CConvert.8b3d5d6a6c17be04:ImplicitAs.Core.b88d1103f417c6d4"(i32 %other), !dbg !15
+// CHECK:STDOUT:   %2 = load i32, ptr %self, align 4, !dbg !16
+// CHECK:STDOUT:   %3 = add i32 %2, %1, !dbg !16
+// CHECK:STDOUT:   store i32 %3, ptr %self, align 4, !dbg !16
+// CHECK:STDOUT:   ret void, !dbg !16
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: define linkonce_odr i32 @"_CConvert.8b3d5d6a6c17be04:ImplicitAs.Core.b88d1103f417c6d4"(i32 %self) !dbg !17 {
+// CHECK:STDOUT:   ret i32 %self, !dbg !19
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: attributes #0 = { nocallback nofree nosync nounwind willreturn memory(argmem: readwrite) }
+// CHECK:STDOUT:
+// CHECK:STDOUT: !llvm.module.flags = !{!0, !1}
+// CHECK:STDOUT: !llvm.dbg.cu = !{!2}
+// CHECK:STDOUT:
+// CHECK:STDOUT: !0 = !{i32 7, !"Dwarf Version", i32 5}
+// CHECK:STDOUT: !1 = !{i32 2, !"Debug Info Version", i32 3}
+// CHECK:STDOUT: !2 = distinct !DICompileUnit(language: DW_LANG_C, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
+// CHECK:STDOUT: !3 = !DIFile(filename: "increment.carbon", directory: "")
+// CHECK:STDOUT: !4 = distinct !DISubprogram(name: "IncrSigned", linkageName: "_CIncrSigned.Main", scope: null, file: !3, line: 4, type: !5, spFlags: DISPFlagDefinition, unit: !2)
+// CHECK:STDOUT: !5 = !DISubroutineType(types: !6)
+// CHECK:STDOUT: !6 = !{}
+// CHECK:STDOUT: !7 = !DILocation(line: 5, column: 3, scope: !4)
+// CHECK:STDOUT: !8 = !DILocation(line: 6, column: 3, scope: !4)
+// CHECK:STDOUT: !9 = !DILocation(line: 4, column: 1, scope: !4)
+// CHECK:STDOUT: !10 = distinct !DISubprogram(name: "Op", linkageName: "_COp.Int.Core:Inc.Core.be1e879c1ad406d8", scope: null, file: !11, line: 331, type: !5, spFlags: DISPFlagDefinition, unit: !2)
+// CHECK:STDOUT: !11 = !DIFile(filename: "{{.*}}/prelude/types/int.carbon", directory: "")
+// CHECK:STDOUT: !12 = !DILocation(line: 333, column: 5, scope: !10)
+// CHECK:STDOUT: !13 = !DILocation(line: 331, column: 3, scope: !10)
+// CHECK:STDOUT: !14 = distinct !DISubprogram(name: "Op", linkageName: "_COp:thunk.Int.Core:AddAssignWith.Core.25a9a5e901f5b032", scope: null, file: !11, line: 267, type: !5, spFlags: DISPFlagDefinition, unit: !2)
+// CHECK:STDOUT: !15 = !DILocation(line: 4294967295, scope: !14)
+// CHECK:STDOUT: !16 = !DILocation(line: 267, column: 3, scope: !14)
+// CHECK:STDOUT: !17 = distinct !DISubprogram(name: "Convert", linkageName: "_CConvert.8b3d5d6a6c17be04:ImplicitAs.Core.b88d1103f417c6d4", scope: null, file: !18, line: 18, type: !5, spFlags: DISPFlagDefinition, unit: !2)
+// CHECK:STDOUT: !18 = !DIFile(filename: "{{.*}}/prelude/operators/as.carbon", directory: "")
+// CHECK:STDOUT: !19 = !DILocation(line: 18, column: 38, scope: !17)