[globalisel][irtranslator] Verify that DILocations aren't lost in translation

[dsanders]

Summary

Also fix a couple bugs where DILocations are lost. EntryBuilder wasn't passing
on debug locations for PHI's, constants, GLOBAL_VALUE, etc.

[vsk]

The verifier bits look great, I'll let someone else comment on the functionality changes. Thanks!

test/CodeGen/AArch64/GlobalISel/irtranslator-dilocation.ll

33

It's probably safe to prune all of these out.

[aemerson]

lib/CodeGen/GlobalISel/IRTranslator.cpp

124

I think this debug print is probably unnecessary given it'll print for every instruction translated (and without ASSERTIONS it won't actually do anything).

1742

This is unnecessary as Verifier goes out of scope immediately after?

[dsanders]

lib/CodeGen/GlobalISel/IRTranslator.cpp

124

I'm not sure I understand this comment. It sounds like you're suggesting its removal but the follow on implies the right thing to do is to wrap it in '#ifndef NDEBUG'.

1742

Good point

[aemerson]

lib/CodeGen/GlobalISel/IRTranslator.cpp

124

Yes I was suggesting removing it.

AFAIK NDEBUG and assertions are independent of each other, so without assertions enabled this will print a message that doesn't actually check anything. Could be wrong about that.

[dsanders]

lib/CodeGen/GlobalISel/IRTranslator.cpp

124

I'd prefer not to remove it as it was quite useful to see what DILocations were expected to go where when I was debugging the missing DILocations that are fixed in this patch. I do agree it should only print "Checking ..." when the checks are actually happening though.

AFAIK NDEBUG and assertions are independent of each other, so without assertions enabled this will
print a message that doesn't actually check anything. Could be wrong about that.

The effects of LLVM_ENABLE_ASSERTIONS are achieved by adding -UNDEBUG and removing any -DNDEBUG from the standard build flags. The code for that is in HandleLLVMOptions.cmake

[aemerson]

LGTM with the nits addressed.

lib/CodeGen/GlobalISel/IRTranslator.cpp

static void reportTranslationError(MachineFunction &MF,
   else
     ORE.emit(R);
 }
 
 IRTranslator::IRTranslator() : MachineFunctionPass(ID) {
   initializeIRTranslatorPass(*PassRegistry::getPassRegistry());
 }
 
+#ifndef NDEBUG
+/// Verify that every instruction created has the same DILocation as the
+/// instruction being translated.
+class DILocationVerifier : MachineFunction::Delegate {
+  MachineFunction &MF;
+  const Instruction *CurrInst = nullptr;
+
+public:
+  DILocationVerifier(MachineFunction &MF) : MF(MF) { MF.setDelegate(this); }
+  ~DILocationVerifier() { MF.resetDelegate(this); }
+
+  const Instruction *getCurrentInst() const { return CurrInst; }
+  void setCurrentInst(const Instruction *Inst) { CurrInst = Inst; }
+
+  void MF_HandleInsertion(const MachineInstr &MI) override {
+    assert(getCurrentInst() &&
+           "Inserted instruction without a current MI");
+    LLVM_DEBUG(dbgs() << "Checking DILocation from " << *CurrInst
+                      << " was copied to " << MI);
+    assert(CurrInst->getDebugLoc() == MI.getDebugLoc() &&
+           "Line info was not transferred to all instructions");
+  }
+  void MF_HandleRemoval(const MachineInstr &MI) override {}
+};
+#endif // ifndef NDEBUG
+
+
 void IRTranslator::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<StackProtector>();
   AU.addRequired<TargetPassConfig>();
   getSelectionDAGFallbackAnalysisUsage(AU);
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
 static void computeValueLLTs(const DataLayout &DL, Type &Ty,
MIRBuilder.buildAtomicRMW(
       *MF->getMachineMemOperand(MachinePointerInfo(I.getPointerOperand()),
                                 Flags, DL->getTypeStoreSize(ResType),
                                 getMemOpAlignment(I), AAMDNodes(), nullptr,
                                 I.getSyncScopeID(), I.getOrdering()));
   return true;
 }
 
 void IRTranslator::finishPendingPhis() {
+#ifndef NDEBUG
+  DILocationVerifier Verifier(*MF);
+#endif // ifndef NDEBUG
   for (auto &Phi : PendingPHIs) {
     const PHINode *PI = Phi.first;
     ArrayRef<MachineInstr *> ComponentPHIs = Phi.second;
+    EntryBuilder.setDebugLoc(PI->getDebugLoc());
+#ifndef NDEBUG
+    Verifier.setCurrentInst(PI);
+#endif // ifndef NDEBUG
 
     // All MachineBasicBlocks exist, add them to the PHI. We assume IRTranslator
     // won't create extra control flow here, otherwise we need to find the
     // dominating predecessor here (or perhaps force the weirder IRTranslators
     // to provide a simple boundary).
     SmallSet<const BasicBlock *, 4> HandledPreds;
 
     for (unsigned i = 0; i < PI->getNumIncomingValues(); ++i) {
bool IRTranslator::valueIsSplit(const Value &V,
   if (Offsets && !Offsets->empty())
     Offsets->clear();
   computeValueLLTs(*DL, *V.getType(), SplitTys, Offsets);
   return SplitTys.size() > 1;
 }
 
 bool IRTranslator::translate(const Instruction &Inst) {
   CurBuilder.setDebugLoc(Inst.getDebugLoc());
+  EntryBuilder.setDebugLoc(Inst.getDebugLoc());
   switch(Inst.getOpcode()) {
 #define HANDLE_INST(NUM, OPCODE, CLASS) \
     case Instruction::OPCODE: return translate##OPCODE(Inst, CurBuilder);
 #include "llvm/IR/Instruction.def"
   default:
     return false;
   }
 }
if (!valueIsSplit(*ArgIt, VMap.getOffsets(*ArgIt))) {
       VRegs.push_back(VArg);
     } else {
       unpackRegs(*ArgIt, VArg, EntryBuilder);
     }
     ArgIt++;
   }
 
   // Need to visit defs before uses when translating instructions.
+  {
     ReversePostOrderTraversal<const Function *> RPOT(&F);
+#ifndef NDEBUG
+    DILocationVerifier Verifier(*MF);
+#endif // ifndef NDEBUG
     for (const BasicBlock *BB : RPOT) {
       MachineBasicBlock &MBB = getMBB(*BB);
       // Set the insertion point of all the following translations to
       // the end of this basic block.
       CurBuilder.setMBB(MBB);
 
       for (const Instruction &Inst : *BB) {
+#ifndef NDEBUG
+        Verifier.setCurrentInst(&Inst);
+#endif // ifndef NDEBUG
         if (translate(Inst))
           continue;
 
         OptimizationRemarkMissed R("gisel-irtranslator", "GISelFailure",
                                    Inst.getDebugLoc(), BB);
         R << "unable to translate instruction: " << ore::NV("Opcode", &Inst);
 
         if (ORE->allowExtraAnalysis("gisel-irtranslator")) {
           std::string InstStrStorage;
           raw_string_ostream InstStr(InstStrStorage);
           InstStr << Inst;
 
           R << ": '" << InstStr.str() << "'";
         }
 
         reportTranslationError(*MF, *TPC, *ORE, R);
         return false;
       }
     }
+#ifndef NDEBUG
+    Verifier.setCurrentInst(nullptr);
+#endif // ifndef NDEBUG
+  }
 
   finishPendingPhis();
 
   // Merge the argument lowering and constants block with its single
   // successor, the LLVM-IR entry block.  We want the basic block to
   // be maximal.
   assert(EntryBB->succ_size() == 1 &&
          "Custom BB used for lowering should have only one successor");

test/CodeGen/AArch64/GlobalISel/irtranslator-dilocation.ll

+; RUN: llc -O0 -mtriple=aarch64-apple-ios -global-isel -debug-only=irtranslator \
+; RUN:     -stop-after=irtranslator %s -o - 2>&1 | FileCheck %s
+
+; CHECK: Checking DILocation from   %retval = alloca i32, align 4 was copied to G_FRAME_INDEX
+; CHECK: Checking DILocation from   %rv = alloca i32, align 4 was copied to G_FRAME_INDEX
+; CHECK: Checking DILocation from   store i32 0, i32* %retval, align 4 was copied to G_CONSTANT
+; CHECK: Checking DILocation from   store i32 0, i32* %retval, align 4 was copied to G_STORE
+; CHECK: Checking DILocation from   store i32 0, i32* %rv, align 4, !dbg !12 was copied to G_STORE debug-location !12; t.cpp:2:5
+; CHECK: Checking DILocation from   %0 = load i32, i32* %rv, align 4, !dbg !13 was copied to G_LOAD debug-location !13; t.cpp:3:8
+; CHECK: Checking DILocation from   ret i32 %0, !dbg !14 was copied to COPY debug-location !14; t.cpp:3:1
+; CHECK: Checking DILocation from   ret i32 %0, !dbg !14 was copied to RET_ReallyLR implicit $w0, debug-location !14; t.cpp:3:1
+
+source_filename = "t.cpp"
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "arm64-unknown-linux-gnu"
+
+; Function Attrs: noinline norecurse nounwind optnone
+define dso_local i32 @main() #0 !dbg !7 {
+entry:
+  %retval = alloca i32, align 4
+  %rv = alloca i32, align 4
+  store i32 0, i32* %retval, align 4
+  call void @llvm.dbg.declare(metadata i32* %rv, metadata !11, metadata !DIExpression()), !dbg !12
+  store i32 0, i32* %rv, align 4, !dbg !12
+  %0 = load i32, i32* %rv, align 4, !dbg !13
+  ret i32 %0, !dbg !14
+}
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline norecurse nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+neon" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 8.0.0 (trunk) (llvm/trunk 344296)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "t.cpp", directory: "/Volumes/Data/llvm.org/svn/build")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 8.0.0 (trunk) (llvm/trunk 344296)"}
+!7 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !DILocalVariable(name: "rv", scope: !7, file: !1, line: 2, type: !10)
+!12 = !DILocation(line: 2, column: 5, scope: !7)
+!13 = !DILocation(line: 3, column: 8, scope: !7)
+!14 = !DILocation(line: 3, column: 1, scope: !7)
+