[llvm] r245589 - Fix a bug that caused SimplifyCFG to drop DebugLocs.
Eric Christopher via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 20 13:54:18 PDT 2015
On Thu, Aug 20, 2015 at 1:52 PM Adrian Prantl via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> On Aug 20, 2015, at 11:38 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Thu, Aug 20, 2015 at 11:24 AM, Adrian Prantl via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: adrian
>> Date: Thu Aug 20 13:24:02 2015
>> New Revision: 245589
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=245589&view=rev
>> <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=>
>> Log:
>> Fix a bug that caused SimplifyCFG to drop DebugLocs.
>>
>> Instruction::dropUnknownMetadata(KnownSet) is supposed to preserve all
>> metadata in KnownSet, but the condition for DebugLocs was inverted.
>>
>> Most users of dropUnknownMetadata() actually worked around this by not
>> adding LLVMContext::MD_dbg to their list of KnowIDs.
>> This is now made explicit.
>>
>
> 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?
>
>
> 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.
>
>
"Some degree". Probably not intentional would be my guess though.
> If we change the API we should probably rename the function to take the
> debug info location preserving into account.
>
>
I think always preserving would be the right way to go.
-eric
> -- adrian
>
>
> & 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)
>
>
>>
>> Modified:
>> llvm/trunk/lib/IR/Metadata.cpp
>> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>> llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>> llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
>> llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
>> llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>> llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp
>> llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll
>>
>> Modified: llvm/trunk/lib/IR/Metadata.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Metadata.cpp?rev=245589&r1=245588&r2=245589&view=diff
>> <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=>
>>
>> ==============================================================================
>> --- llvm/trunk/lib/IR/Metadata.cpp (original)
>> +++ llvm/trunk/lib/IR/Metadata.cpp Thu Aug 20 13:24:02 2015
>> @@ -1062,7 +1062,7 @@ void Instruction::dropUnknownMetadata(Ar
>> KnownSet.insert(KnownIDs.begin(), KnownIDs.end());
>>
>> // Drop debug if needed
>> - if (KnownSet.erase(LLVMContext::MD_dbg))
>> + if (!KnownSet.erase(LLVMContext::MD_dbg))
>> DbgLoc = DebugLoc();
>>
>> if (!hasMetadataHashEntry())
>>
>> Modified:
>> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=245589&r1=245588&r2=245589&view=diff
>> <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=>
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>> (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>> Thu Aug 20 13:24:02 2015
>> @@ -754,6 +754,7 @@ Instruction *InstCombiner::visitLoadInst
>> 6, AA, &AATags)) {
>> if (LoadInst *NLI = dyn_cast<LoadInst>(AvailableVal)) {
>> unsigned KnownIDs[] = {
>> + LLVMContext::MD_dbg,
>> LLVMContext::MD_tbaa,
>> LLVMContext::MD_alias_scope,
>> LLVMContext::MD_noalias,
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=245589&r1=245588&r2=245589&view=diff
>> <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=>
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Thu Aug 20 13:24:02 2015
>> @@ -1780,6 +1780,7 @@ static void patchReplacementInstruction(
>> // regions, and so we need a conservative combination of the noalias
>> // scopes.
>> static const unsigned KnownIDs[] = {
>> + LLVMContext::MD_dbg,
>> LLVMContext::MD_tbaa,
>> LLVMContext::MD_alias_scope,
>> LLVMContext::MD_noalias,
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp?rev=245589&r1=245588&r2=245589&view=diff
>> <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=>
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp Thu Aug 20
>> 13:24:02 2015
>> @@ -743,6 +743,7 @@ bool MemCpyOpt::performCallSlotOptzn(Ins
>> // FIXME: MD_tbaa_struct and MD_mem_parallel_loop_access should also be
>> // handled here, but combineMetadata doesn't support them yet
>> unsigned KnownIDs[] = {
>> + LLVMContext::MD_dbg,
>> LLVMContext::MD_tbaa,
>> LLVMContext::MD_alias_scope,
>> LLVMContext::MD_noalias,
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp?rev=245589&r1=245588&r2=245589&view=diff
>> <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=>
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp Thu Aug 20
>> 13:24:02 2015
>> @@ -293,7 +293,7 @@ void MergedLoadStoreMotion::hoistInstruc
>>
>> // Intersect optional metadata.
>> HoistCand->intersectOptionalDataWith(ElseInst);
>> - HoistCand->dropUnknownMetadata();
>> + HoistCand->dropUnknownMetadata(LLVMContext::MD_dbg);
>>
>> // Prepend point for instruction insert
>> Instruction *HoistPt = BB->getTerminator();
>> @@ -472,7 +472,7 @@ bool MergedLoadStoreMotion::sinkStore(Ba
>> BasicBlock::iterator InsertPt = BB->getFirstInsertionPt();
>> // Intersect optional metadata.
>> S0->intersectOptionalDataWith(S1);
>> - S0->dropUnknownMetadata();
>> + S0->dropUnknownMetadata(LLVMContext::MD_dbg);
>>
>> // Create the new store to be inserted at the join point.
>> StoreInst *SNew = (StoreInst *)(S0->clone());
>>
>> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=245589&r1=245588&r2=245589&view=diff
>> <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=>
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Thu Aug 20 13:24:02
>> 2015
>> @@ -1093,6 +1093,7 @@ static bool HoistThenElseCodeToIf(Branch
>> I2->replaceAllUsesWith(I1);
>> I1->intersectOptionalDataWith(I2);
>> unsigned KnownIDs[] = {
>> + LLVMContext::MD_dbg,
>> LLVMContext::MD_tbaa,
>> LLVMContext::MD_range,
>> LLVMContext::MD_fpmath,
>>
>> Modified: llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp?rev=245589&r1=245588&r2=245589&view=diff
>> <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=>
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp Thu Aug 20
>> 13:24:02 2015
>> @@ -3118,6 +3118,7 @@ namespace {
>> K->mutateType(getVecTypeForPair(L->getType(), H->getType()));
>>
>> unsigned KnownIDs[] = {
>> + LLVMContext::MD_dbg,
>> LLVMContext::MD_tbaa,
>> LLVMContext::MD_alias_scope,
>> LLVMContext::MD_noalias,
>>
>> Modified: llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll?rev=245589&r1=245588&r2=245589&view=diff
>> <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=>
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll (original)
>> +++ llvm/trunk/test/Transforms/SimplifyCFG/basictest.ll Thu Aug 20
>> 13:24:02 2015
>> @@ -50,7 +50,7 @@ define i8 @test6f() {
>> ; CHECK: alloca i8, align 1
>> ; CHECK-NEXT: call i8 @test6g
>> ; CHECK-NEXT: icmp eq i8 %tmp, 0
>> -; CHECK-NEXT: load i8, i8* %r, align 1{{$}}
>> +; CHECK-NEXT: load i8, i8* %r, align 1, !dbg !{{[0-9]+$}}
>>
>> bb0:
>> %r = alloca i8, align 1
>> @@ -58,7 +58,7 @@ bb0:
>> %tmp1 = icmp eq i8 %tmp, 0
>> br i1 %tmp1, label %bb2, label %bb1
>> bb1:
>> - %tmp3 = load i8, i8* %r, align 1, !range !2, !tbaa !1
>> + %tmp3 = load i8, i8* %r, align 1, !range !2, !tbaa !1, !dbg !5
>> %tmp4 = icmp eq i8 %tmp3, 1
>> br i1 %tmp4, label %bb2, label %bb3
>> bb2:
>> @@ -69,6 +69,16 @@ bb3:
>> }
>> declare i8 @test6g(i8*)
>>
>> +!llvm.dbg.cu
>> <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=>
>> = !{!3}
>> +!llvm.module.flags = !{!8, !9}
>> +
>> !0 = !{!1, !1, i64 0}
>> !1 = !{!"foo"}
>> !2 = !{i8 0, i8 2}
>> +!3 = distinct !DICompileUnit(language: DW_LANG_C99, file: !7, producer:
>> "clang", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !4,
>> subprograms: !4, globals: !4)
>> +!4 = !{}
>> +!5 = !DILocation(line: 23, scope: !6)
>> +!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)
>> +!7 = !DIFile(filename: "foo.c", directory: "/")
>> +!8 = !{i32 2, !"Dwarf Version", i32 2}
>> +!9 = !{i32 2, !"Debug Info Version", i32 3}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> <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=>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150820/94a7c79e/attachment.html>
More information about the llvm-commits
mailing list