<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Aug 20, 2015 at 1:52 PM Adrian Prantl via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><blockquote type="cite"><div>On Aug 20, 2015, at 11:38 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><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="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D245589-26view-3Drev&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=EtKQ5g1zzu-b3OZrJlxcjqb4iXC6E05iJjC_rMoboF0&e=" 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></div></div></div></div></div></blockquote><div><br></div></div></div><div style="word-wrap:break-word"><div><div>There must have been some degree of deliberation involved: after I fixed the condition there was at least one testcase that failed because an optimization had dropped the DebugLoc of a dbg.value intrinsic, which is illegal. Good, maybe the assertion was added later.</div><div><br></div></div></div></blockquote><div><br></div><div>"Some degree". Probably not intentional would be my guess though.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div></div><div>If we change the API we should probably rename the function to take the debug info location preserving into account.</div></div></div><div style="word-wrap:break-word"><div><div><br></div></div></div></blockquote><div><br></div><div>I think always preserving would be the right way to go.</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div></div>-- adrian</div></div><div style="word-wrap:break-word"><div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><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="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_IR_Metadata.cpp-3Frev-3D245589-26r1-3D245588-26r2-3D245589-26view-3Ddiff&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=wzZSN6kqTDDDoW7l2jw9R54F1utkPape8AnlfvGqa1I&e=" 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="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_InstCombine_InstCombineLoadStoreAlloca.cpp-3Frev-3D245589-26r1-3D245588-26r2-3D245589-26view-3Ddiff&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=6KlcQlv4NcZd8p1JUrb_I1gvJJrO8nshSHv-mGcHfIQ&e=" 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="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_Scalar_GVN.cpp-3Frev-3D245589-26r1-3D245588-26r2-3D245589-26view-3Ddiff&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=CBzA4N3fiGql6aJLY83wxjjajWLuo3IGiOFMzwHt-V8&e=" 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="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_Scalar_MemCpyOptimizer.cpp-3Frev-3D245589-26r1-3D245588-26r2-3D245589-26view-3Ddiff&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=aRPTmlmNnQD_2UTS0yuoxPUVNVsDdokceWyM5-VSwNY&e=" 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="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_Scalar_MergedLoadStoreMotion.cpp-3Frev-3D245589-26r1-3D245588-26r2-3D245589-26view-3Ddiff&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=y5vdyH-S9iOgEU9D0l36UeyVwqjPvibDxNcp8xdjLHI&e=" 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="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_Utils_SimplifyCFG.cpp-3Frev-3D245589-26r1-3D245588-26r2-3D245589-26view-3Ddiff&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=MlMYlre4jafLQGTOCiFDPPtUkr24e5VUAXH_INIvEp8&e=" 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="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_Vectorize_BBVectorize.cpp-3Frev-3D245589-26r1-3D245588-26r2-3D245589-26view-3Ddiff&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=xBoZzt_2qdEo5pD0DVxVM874Kns14XA0Tcc_zV1Qyt4&e=" 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="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_Transforms_SimplifyCFG_basictest.ll-3Frev-3D245589-26r1-3D245588-26r2-3D245589-26view-3Ddiff&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=OoNwP-iPrNBLLZtCDo4QBCA-2NZq8A0UX9r8ARqy8Vc&e=" 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="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.dbg.cu&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=_YwAfZKuMj-3b7_5-qtsYdjH6-LxIYe-tYY9ywlPNT0&e=" 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" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=BQMFaQ&c=eEvniauFctOgLOKGJOplqw&r=cTx6f1tAfqPeajYunFWp7_8ot79RnHyNteqzig4fXmA&m=O5hdC-rwRMOUzsKRZ7x6owaIS6MAd-YtKndGVMJjajI&s=B81xk9guP5NgjYvMbPIeEnddffTrqv_-51j1Eu4RO0c&e=" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>
</div></blockquote></div></div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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></div>