[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 11:38:33 PDT 2015


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
> 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?

& 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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 = !{!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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150820/9264beaf/attachment.html>


More information about the llvm-commits mailing list