[llvm] r245589 - Fix a bug that caused SimplifyCFG to drop DebugLocs.
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 20 14:41:01 PDT 2015
SGTM. We're only dealing with 14 instances of it anyway.
On Thu, Aug 20, 2015 at 2:30 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
> On Aug 20, 2015, at 1:54 PM, Eric Christopher <echristo at gmail.com> wrote:
>
>
>
> 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.
>
>
> I we’re doing this, I suggest renaming the function to something like
> dropUnknownNonDebugMetadata() to avoid confusion about what it does.
> Any better ideas for descriptive names?
>
> -- adrian
>
>
> -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
>> <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=JtCJPwcGJ3rTAk5LXgRXmZ11Iy9NPKjvbHJI66W-7Bk&s=wLLzXmk9K2UerI-0T9hlbiy_lED6-WOSkiZW-jfLvpA&e=>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150820/1360724e/attachment.html>
More information about the llvm-commits
mailing list