Have stripDebugInfo() also strip !llvm.loop annotations.

[aprantl]

Summary

The !llvm.loop annotations consist of pointers into the debug info, so when stripping the debug info (particularly important when it is malformed!) !llvm.loop annotations need to be stripped as well, or else the malformed debug info stays around.

rdar://73687049

[vsk]

llvm/lib/IR/DebugInfo.cpp

344

We should consider keeping the non-debug info metadata within MD_loop to avoid affecting optimization or opt-remarks. I think we can do that with I.setMD(MD_loop, stripDebugLocFromLoopID(MD)).

llvm/test/Verifier/llvm.loop-cu-strip.ll

2

Can you round-trip this through llvm-dis so we can check that the !llvm.loop metadata sticks around (I'm imagining some CHECK: !5 = distinct !{!5}).

[vsk]

Or, if MD_loop without the debug info doesn't serve any purpose, it'd be fine to drop it entirely. Perhaps @fhahn can speak to this?

[aprantl]

llvm/lib/IR/DebugInfo.cpp

344

I didn't know this was an option! Thanks

[aprantl]

llvm/lib/IR/DebugInfo.cpp

344

This is weird — the code right below is already supposed to do this and I didn't notice!

[aprantl]

llvm/lib/IR/DebugInfo.cpp

344

Because it only looks at TermInsts.

[aprantl]

Address review feedback!

[aprantl]

Lesson learned — we can't depend on the IR being well-formed in the code that strips malformed annotations ;-)

[aprantl]

My assumption is that the annotation shouldn't have been on the call to begin with, but I have bitcode produced by Apple clang-1100 from which this testcase was reduced.

[vsk]

Neat, looks good to me!

llvm/lib/IR/DebugInfo.cpp

for (auto II = BB.begin(), End = BB.end(); II != End;) {
         I.eraseFromParent();
         Changed = true;
         continue;
       }
       if (I.getDebugLoc()) {
         Changed = true;
         I.setDebugLoc(DebugLoc());
       }
-    }
-
-    auto *TermInst = BB.getTerminator();
-    if (!TermInst)
-      // This is invalid IR, but we may not have run the verifier yet
-      continue;
-    if (auto *LoopID = TermInst->getMetadata(LLVMContext::MD_loop)) {
+      if (auto *LoopID = I.getMetadata(LLVMContext::MD_loop)) {
         auto *NewLoopID = LoopIDsMap.lookup(LoopID);
         if (!NewLoopID)
           NewLoopID = LoopIDsMap[LoopID] = stripDebugLocFromLoopID(LoopID);
         if (NewLoopID != LoopID)
-        TermInst->setMetadata(LLVMContext::MD_loop, NewLoopID);
+          I.setMetadata(LLVMContext::MD_loop, NewLoopID);
+      }
     }
   }
   return Changed;
 }
 
 bool llvm::StripDebugInfo(Module &M) {
   bool Changed = false;
 

llvm/test/Verifier/llvm.loop-cu-strip.ll

+; RUN: llvm-as -disable-output < %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: llvm-as < %s -o - | llvm-dis - | FileCheck %s --check-prefix=CHECK-STRIP
+; CHECK: DICompileUnit not listed in llvm.dbg.cu
+; CHECK: ignoring invalid debug info in
+; CHECK-NOT: DICompileUnit not listed in llvm.dbg.cu
+declare hidden void @g() local_unnamed_addr #1 align 2
+define hidden void @f() {
+  tail call void @g() #2, !llvm.loop !5
+  ret void
+}
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 2, !"Dwarf Version", i32 4}
+!1 = !{i32 2, !"Debug Info Version", i32 3}
+; CHECK-STRIP: ![[MD:.*]] = distinct !{![[MD]], !"fake loop metadata"}
+!5 = distinct !{!5, !6, !6, !"fake loop metadata"}
+!6 = !DILocation(line: 1325, column: 3, scope: !7)
+!7 = distinct !DISubprogram(name: "f", scope: !8, file: !8, line: 1324, type: !9, scopeLine: 1324, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !11)
+!8 = !DIFile(filename: "/", directory: "f.cpp")
+!9 = !DISubroutineType(types: !10)
+!10 = !{}
+!11 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !8)