<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 20, 2015 at 11:24 AM, Adrian Prantl via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: adrian<br>
Date: Thu Aug 20 13:24:02 2015<br>
New Revision: 245589<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=245589&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=245589&view=rev</a><br>
Log:<br>
Fix a bug that caused SimplifyCFG to drop DebugLocs.<br>
<br>
Instruction::dropUnknownMetadata(KnownSet) is supposed to preserve all<br>
metadata in KnownSet, but the condition for DebugLocs was inverted.<br>
<br>
Most users of dropUnknownMetadata() actually worked around this by not<br>
adding LLVMContext::MD_dbg to their list of KnowIDs.<br>
This is now made explicit.<br></blockquote><div><br></div><div>I wonder if this API should just never drop dbg? (were any callers actually using it to drop dbg data) I imagine those existing callers came about not by deliberately working around the weird behavior, but by simply omitting dbg because they didn't think they needed to have it?<br><br>& new callers could arise with the same assumption, but now they'll be (pretty silently) wrong... (but that's just the general problem of optimization passes not being terribly aware of debug info preservation/propagation)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified:<br>
    llvm/trunk/lib/IR/Metadata.cpp<br>
    llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp<br>
    llvm/trunk/lib/Transforms/Scalar/GVN.cpp<br>
    llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp<br>
    llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp<br>
    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp<br>
    llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp<br>
    llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll<br>
<br>
Modified: llvm/trunk/lib/IR/Metadata.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=245589&r1=245588&r2=245589&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=245589&r1=245588&r2=245589&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/Metadata.cpp (original)<br>
+++ llvm/trunk/lib/IR/Metadata.cpp Thu Aug 20 13:24:02 2015<br>
@@ -1062,7 +1062,7 @@ void Instruction::dropUnknownMetadata(Ar<br>
   KnownSet.insert(KnownIDs.begin(), KnownIDs.end());<br>
<br>
   // Drop debug if needed<br>
-  if (KnownSet.erase(LLVMContext::MD_dbg))<br>
+  if (!KnownSet.erase(LLVMContext::MD_dbg))<br>
     DbgLoc = DebugLoc();<br>
<br>
   if (!hasMetadataHashEntry())<br>
<br>
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=245589&r1=245588&r2=245589&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=245589&r1=245588&r2=245589&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp Thu Aug 20 13:24:02 2015<br>
@@ -754,6 +754,7 @@ Instruction *InstCombiner::visitLoadInst<br>
                                                      6, AA, &AATags)) {<br>
     if (LoadInst *NLI = dyn_cast<LoadInst>(AvailableVal)) {<br>
       unsigned KnownIDs[] = {<br>
+        LLVMContext::MD_dbg,<br>
         LLVMContext::MD_tbaa,<br>
         LLVMContext::MD_alias_scope,<br>
         LLVMContext::MD_noalias,<br>
<br>
Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=245589&r1=245588&r2=245589&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=245589&r1=245588&r2=245589&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Thu Aug 20 13:24:02 2015<br>
@@ -1780,6 +1780,7 @@ static void patchReplacementInstruction(<br>
     // regions, and so we need a conservative combination of the noalias<br>
     // scopes.<br>
     static const unsigned KnownIDs[] = {<br>
+      LLVMContext::MD_dbg,<br>
       LLVMContext::MD_tbaa,<br>
       LLVMContext::MD_alias_scope,<br>
       LLVMContext::MD_noalias,<br>
<br>
Modified: llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp?rev=245589&r1=245588&r2=245589&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp?rev=245589&r1=245588&r2=245589&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp Thu Aug 20 13:24:02 2015<br>
@@ -743,6 +743,7 @@ bool MemCpyOpt::performCallSlotOptzn(Ins<br>
   // FIXME: MD_tbaa_struct and MD_mem_parallel_loop_access should also be<br>
   // handled here, but combineMetadata doesn't support them yet<br>
   unsigned KnownIDs[] = {<br>
+    LLVMContext::MD_dbg,<br>
     LLVMContext::MD_tbaa,<br>
     LLVMContext::MD_alias_scope,<br>
     LLVMContext::MD_noalias,<br>
<br>
Modified: llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp?rev=245589&r1=245588&r2=245589&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp?rev=245589&r1=245588&r2=245589&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp Thu Aug 20 13:24:02 2015<br>
@@ -293,7 +293,7 @@ void MergedLoadStoreMotion::hoistInstruc<br>
<br>
   // Intersect optional metadata.<br>
   HoistCand->intersectOptionalDataWith(ElseInst);<br>
-  HoistCand->dropUnknownMetadata();<br>
+  HoistCand->dropUnknownMetadata(LLVMContext::MD_dbg);<br>
<br>
   // Prepend point for instruction insert<br>
   Instruction *HoistPt = BB->getTerminator();<br>
@@ -472,7 +472,7 @@ bool MergedLoadStoreMotion::sinkStore(Ba<br>
     BasicBlock::iterator InsertPt = BB->getFirstInsertionPt();<br>
     // Intersect optional metadata.<br>
     S0->intersectOptionalDataWith(S1);<br>
-    S0->dropUnknownMetadata();<br>
+    S0->dropUnknownMetadata(LLVMContext::MD_dbg);<br>
<br>
     // Create the new store to be inserted at the join point.<br>
     StoreInst *SNew = (StoreInst *)(S0->clone());<br>
<br>
Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=245589&r1=245588&r2=245589&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=245589&r1=245588&r2=245589&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Thu Aug 20 13:24:02 2015<br>
@@ -1093,6 +1093,7 @@ static bool HoistThenElseCodeToIf(Branch<br>
       I2->replaceAllUsesWith(I1);<br>
     I1->intersectOptionalDataWith(I2);<br>
     unsigned KnownIDs[] = {<br>
+      LLVMContext::MD_dbg,<br>
       LLVMContext::MD_tbaa,<br>
       LLVMContext::MD_range,<br>
       LLVMContext::MD_fpmath,<br>
<br>
Modified: llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp?rev=245589&r1=245588&r2=245589&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp?rev=245589&r1=245588&r2=245589&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp Thu Aug 20 13:24:02 2015<br>
@@ -3118,6 +3118,7 @@ namespace {<br>
         K->mutateType(getVecTypeForPair(L->getType(), H->getType()));<br>
<br>
       unsigned KnownIDs[] = {<br>
+        LLVMContext::MD_dbg,<br>
         LLVMContext::MD_tbaa,<br>
         LLVMContext::MD_alias_scope,<br>
         LLVMContext::MD_noalias,<br>
<br>
Modified: llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll?rev=245589&r1=245588&r2=245589&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll?rev=245589&r1=245588&r2=245589&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll (original)<br>
+++ llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll Thu Aug 20 13:24:02 2015<br>
@@ -50,7 +50,7 @@ define i8 @test6f() {<br>
 ; CHECK: alloca i8, align 1<br>
 ; CHECK-NEXT: call i8 @test6g<br>
 ; CHECK-NEXT: icmp eq i8 %tmp, 0<br>
-; CHECK-NEXT: load i8, i8* %r, align 1{{$}}<br>
+; CHECK-NEXT: load i8, i8* %r, align 1, !dbg !{{[0-9]+$}}<br>
<br>
 bb0:<br>
   %r = alloca i8, align 1<br>
@@ -58,7 +58,7 @@ bb0:<br>
   %tmp1 = icmp eq i8 %tmp, 0<br>
   br i1 %tmp1, label %bb2, label %bb1<br>
 bb1:<br>
-  %tmp3 = load i8, i8* %r, align 1, !range !2, !tbaa !1<br>
+  %tmp3 = load i8, i8* %r, align 1, !range !2, !tbaa !1, !dbg !5<br>
   %tmp4 = icmp eq i8 %tmp3, 1<br>
   br i1 %tmp4, label %bb2, label %bb3<br>
 bb2:<br>
@@ -69,6 +69,16 @@ bb3:<br>
 }<br>
 declare i8 @test6g(i8*)<br>
<br>
+!<a href="http://llvm.dbg.cu" rel="noreferrer" target="_blank">llvm.dbg.cu</a> = !{!3}<br>
+!llvm.module.flags = !{!8, !9}<br>
+<br>
 !0 = !{!1, !1, i64 0}<br>
 !1 = !{!"foo"}<br>
 !2 = !{i8 0, i8 2}<br>
+!3 = distinct !DICompileUnit(language: DW_LANG_C99, file: !7, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !4, subprograms: !4, globals: !4)<br>
+!4 = !{}<br>
+!5 = !DILocation(line: 23, scope: !6)<br>
+!6 = !DISubprogram(name: "foo", scope: !3, file: !7, line: 1, type: !DISubroutineType(types: !4), isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: false, variables: !4)<br>
+!7 = !DIFile(filename: "foo.c", directory: "/")<br>
+!8 = !{i32 2, !"Dwarf Version", i32 2}<br>
+!9 = !{i32 2, !"Debug Info Version", i32 3}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>