[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 14:44:41 PDT 2015
Yup.
On Thu, Aug 20, 2015 at 2:41 PM David Blaikie <dblaikie at gmail.com> wrote:
> 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/37576b24/attachment.html>
More information about the llvm-commits
mailing list