[llvm] r245589 - Fix a bug that caused SimplifyCFG to drop DebugLocs.
Adrian Prantl via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 20 15:01:46 PDT 2015
*puts on refactoring gloves*
r245622
-- adrian
> On Aug 20, 2015, at 2:44 PM, Eric Christopher <echristo at gmail.com> wrote:
>
> Yup.
>
> On Thu, Aug 20, 2015 at 2:41 PM David Blaikie <dblaikie at gmail.com <mailto: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 <mailto:aprantl at apple.com>> wrote:
>
>> On Aug 20, 2015, at 1:54 PM, Eric Christopher <echristo at gmail.com <mailto: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 <mailto:llvm-commits at lists.llvm.org>> wrote:
>>> On Aug 20, 2015, at 11:38 AM, David Blaikie <dblaikie at gmail.com <mailto: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 <mailto: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 <mailto: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 <mailto: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/b2706bcf/attachment.html>
More information about the llvm-commits
mailing list