[llvm] r348988 - [LoopDeletion] Update debug values after loop deletion.
Björn Pettersson A via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 10 02:03:47 PST 2019
Solved here:
https://reviews.llvm.org/rL350807
> -----Original Message-----
> From: Björn Pettersson A
> Sent: den 9 januari 2019 22:19
> To: Davide Italiano <davide at freebsd.org>; Björn Pettersson A
> <bjorn.a.pettersson at ericsson.com>; llvm-commits <llvm-
> commits at lists.llvm.org>; Mikael Holmén <mikael.holmen at ericsson.com>
> Subject: Re: [llvm] r348988 - [LoopDeletion] Update debug values after
> loop deletion.
>
> Afaict this hasn't been fixed yet. I'll try to remember to fix it
> tomorrow. Because the RUN line still looks incorrect to me.
>
> ________________________________________
> From: llvm-commits <llvm-commits-bounces at lists.llvm.org> on behalf of
> Björn Pettersson A via llvm-commits <llvm-commits at lists.llvm.org>
> Sent: Friday, December 14, 2018 1:52:49 PM
> To: Davide Italiano; llvm-commits
> Subject: RE: [llvm] r348988 - [LoopDeletion] Update debug values after
> loop deletion.
>
> Thanks Davide. The fix seem to work, but...
>
> Isn't the reproducer you added broken?
>
> The run line (test/Transforms/LoopDeletion/crashbc.ll) looks like this:
>
> ; RUN: opt -loop-deletion -o /dev/null
>
> So you haven't specified any input file, so I doubt that your
> reproducer
> detected the problem.
>
> It would had been nice if llvm-lit detected this kind of problem with
> missing input file. Or have I misunderstood things so llvm-lit is
> adding
> the source by itself somehow? I do not think so (I tested by adding
> "-print-after-all" to the RUN line and using "llvm-lit -a", but that
> does not give me any more output unless I also add "%s" to the RUN
> line).
>
> Regards,
> Björn (testing your patch on behalf of Mikael)
>
> > -----Original Message-----
> > From: llvm-commits <llvm-commits-bounces at lists.llvm.org> On Behalf Of
> > Davide Italiano via llvm-commits
> > Sent: den 13 december 2018 20:11
> > To: Mikael Holmén <mikael.holmen at ericsson.com>
> > Cc: llvm-commits <llvm-commits at lists.llvm.org>
> > Subject: Re: [llvm] r348988 - [LoopDeletion] Update debug values
> after
> > loop deletion.
> >
> > r349069
> > On Thu, Dec 13, 2018 at 10:24 AM Davide Italiano <davide at freebsd.org>
> > wrote:
> > >
> > > Fix coming.
> > > On Thu, Dec 13, 2018 at 10:11 AM Davide Italiano
> <davide at freebsd.org>
> > wrote:
> > > >
> > > > Interesting, taking a look at this now.
> > > > On Thu, Dec 13, 2018 at 4:36 AM Mikael Holmén
> > > > <mikael.holmen at ericsson.com> wrote:
> > > > >
> > > > > Hi Davide,
> > > > >
> > > > > I found a case that starts crashing with this commit.
> > > > >
> > > > > opt -o /dev/null reduced.ll -loop-deletion
> > > > >
> > > > > gives me
> > > > >
> > > > > build-all/bin/opt -o /dev/null reduced.ll -loop-deletion
> > > > > opt: ../lib/Bitcode/Writer/ValueEnumerator.cpp:807: void
> > > > > llvm::ValueEnumerator::EnumerateValue(const llvm::Value *):
> > Assertion
> > > > > `!V->getType()->isVoidTy() && "Can't insert void values!"'
> failed.
> > > > > Stack dump:
> > > > > 0. Program arguments: build-all/bin/opt -o /dev/null
> > reduced.ll
> > > > > -loop-deletion
> > > > > 1. Running pass 'Bitcode Writer' on module 'reduced.ll'.
> > > > > #0 0x0000000002241dc4 PrintStackTraceSignalHandler(void*)
> > > > > (build-all/bin/opt+0x2241dc4)
> > > > > #1 0x000000000223fef0 llvm::sys::RunSignalHandlers()
> > > > > (build-all/bin/opt+0x223fef0)
> > > > > #2 0x0000000002242128 SignalHandler(int) (build-
> > all/bin/opt+0x2242128)
> > > > > #3 0x00007fe9a6e38330 __restore_rt
> > > > > (/lib/x86_64-linux-gnu/libpthread.so.0+0x10330)
> > > > > #4 0x00007fe9a5a27c37 gsignal
> > > > > /build/eglibc-ripdx6/eglibc-
> > 2.19/signal/../nptl/sysdeps/unix/sysv/linux/raise.c:56:0
> > > > > #5 0x00007fe9a5a2b028 abort
> > > > > /build/eglibc-ripdx6/eglibc-2.19/stdlib/abort.c:91:0
> > > > > #6 0x00007fe9a5a20bf6 __assert_fail_base
> > > > > /build/eglibc-ripdx6/eglibc-2.19/assert/assert.c:92:0
> > > > > #7 0x00007fe9a5a20ca2 (/lib/x86_64-linux-gnu/libc.so.6+0x2fca2)
> > > > > #8 0x0000000001815b4d
> > llvm::ValueEnumerator::EnumerateValue(llvm::Value
> > > > > const*) (build-all/bin/opt+0x1815b4d)
> > > > > #9 0x0000000001819222
> > > > > llvm::ValueEnumerator::enumerateMetadataImpl(unsigned int,
> > > > > llvm::Metadata const*) (build-all/bin/opt+0x1819222)
> > > > > #10 0x000000000181850c
> > llvm::ValueEnumerator::EnumerateMetadata(unsigned
> > > > > int, llvm::Metadata const*) (build-all/bin/opt+0x181850c)
> > > > > #11 0x0000000001815304
> > > > > llvm::ValueEnumerator::ValueEnumerator(llvm::Module const&,
> bool)
> > > > > (build-all/bin/opt+0x1815304)
> > > > > #12 0x00000000017ed44c (anonymous
> > > > >
> >
> namespace)::ModuleBitcodeWriterBase::ModuleBitcodeWriterBase(llvm::Modu
> l
> > e
> > > > > const&, llvm::StringTableBuilder&, llvm::BitstreamWriter&,
> bool,
> > > > > llvm::ModuleSummaryIndex const*) (build-all/bin/opt+0x17ed44c)
> > > > > #13 0x00000000017de788
> > llvm::BitcodeWriter::writeModule(llvm::Module
> > > > > const&, bool, llvm::ModuleSummaryIndex const*, bool,
> > std::array<unsigned
> > > > > int, 5ul>*) (build-all/bin/opt+0x17de788)
> > > > > #14 0x00000000017e651a llvm::WriteBitcodeToFile(llvm::Module
> > const&,
> > > > > llvm::raw_ostream&, bool, llvm::ModuleSummaryIndex const*,
> bool,
> > > > > std::array<unsigned int, 5ul>*) (build-all/bin/opt+0x17e651a)
> > > > > #15 0x00000000017dd5ee (anonymous
> > > > > namespace)::WriteBitcodePass::runOnModule(llvm::Module&)
> > > > > (build-all/bin/opt+0x17dd5ee)
> > > > > #16 0x0000000001c54cea
> > llvm::legacy::PassManagerImpl::run(llvm::Module&)
> > > > > (build-all/bin/opt+0x1c54cea)
> > > > > #17 0x0000000000786b8b main (build-all/bin/opt+0x786b8b)
> > > > > #18 0x00007fe9a5a12f45 __libc_start_main
> > > > > /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:321:0
> > > > > #19 0x000000000076c14d _start (build-all/bin/opt+0x76c14d)
> > > > > Abort
> > > > >
> > > > > I found this on my out-of-tree target and then I've reduced it
> and
> > it
> > > > > still exposes the original crash I got, but possibly I've
> messed
> > with
> > > > > the metadata so it looks weird now.
> > > > >
> > > > > It still crashes with this patch and not before it though.
> > > > >
> > > > > Regards,
> > > > > Mikael
> > > > >
> > > > > On 12/13/18 12:32 AM, Davide Italiano via llvm-commits wrote:
> > > > > > Author: davide
> > > > > > Date: Wed Dec 12 15:32:35 2018
> > > > > > New Revision: 348988
> > > > > >
> > > > > > URL: http://llvm.org/viewvc/llvm-project?rev=348988&view=rev
> > > > > > Log:
> > > > > > [LoopDeletion] Update debug values after loop deletion.
> > > > > >
> > > > > > When loops are deleted, we don't keep track of variables
> > modified inside
> > > > > > the loops, so the DI will contain the wrong value for these.
> > > > > >
> > > > > > e.g.
> > > > > >
> > > > > > int b() {
> > > > > >
> > > > > > int i;
> > > > > > for (i = 0; i < 2; i++)
> > > > > > ;
> > > > > > patatino();
> > > > > > return a;
> > > > > > -> 6 patatino();
> > > > > >
> > > > > > 7 return a;
> > > > > > 8 }
> > > > > > 9 int main() { b(); }
> > > > > > (lldb) frame var i
> > > > > > (int) i = 0
> > > > > >
> > > > > > We mark instead these values as unavailable inserting a
> > > > > > @llvm.dbg.value(undef to make sure we don't end up printing
> an
> > incorrect
> > > > > > value in the debugger. We could consider doing something
> > fancier,
> > > > > > for, e.g. constants, in the future.
> > > > > >
> > > > > > PR39868.
> > > > > > rdar://problem/46418795)
> > > > > >
> > > > > > Differential Revision: https://reviews.llvm.org/D55299
> > > > > >
> > > > > > Added:
> > > > > > llvm/trunk/test/Transforms/LoopDeletion/diundef.ll
> > > > > > Modified:
> > > > > > llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp
> > > > > >
> > > > > > Modified: llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp
> > > > > > URL: http://llvm.org/viewvc/llvm-
> >
> project/llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp?rev=348988&r1=348
> 9
> > 87&r2=348988&view=diff
> > > > > >
> >
> =======================================================================
> =
> > ======
> > > > > > --- llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp (original)
> > > > > > +++ llvm/trunk/lib/Transforms/Utils/LoopUtils.cpp Wed Dec 12
> > 15:32:35 2018
> > > > > > @@ -26,9 +26,11 @@
> > > > > > #include "llvm/Analysis/ScalarEvolutionExpressions.h"
> > > > > > #include "llvm/Analysis/TargetTransformInfo.h"
> > > > > > #include "llvm/Analysis/ValueTracking.h"
> > > > > > +#include "llvm/IR/DIBuilder.h"
> > > > > > #include "llvm/IR/DomTreeUpdater.h"
> > > > > > #include "llvm/IR/Dominators.h"
> > > > > > #include "llvm/IR/Instructions.h"
> > > > > > +#include "llvm/IR/IntrinsicInst.h"
> > > > > > #include "llvm/IR/Module.h"
> > > > > > #include "llvm/IR/PatternMatch.h"
> > > > > > #include "llvm/IR/ValueHandle.h"
> > > > > > @@ -567,6 +569,12 @@ void llvm::deleteDeadLoop(Loop *L, Domin
> > > > > > DTU.deleteEdge(Preheader, L->getHeader());
> > > > > > }
> > > > > >
> > > > > > + // Use a map to unique and a vector to guarantee
> > deterministic ordering.
> > > > > > + llvm::SmallDenseMap<std::pair<DIVariable *, DIExpression
> *>,
> > > > > > + DbgVariableIntrinsic *, 4>
> > > > > > + DeadDebugMap;
> > > > > > + llvm::SmallVector<DbgVariableIntrinsic *, 4>
> DeadDebugInst;
> > > > > > +
> > > > > > // Given LCSSA form is satisfied, we should not have
> users
> > of instructions
> > > > > > // within the dead loop outside of the loop. However,
> LCSSA
> > doesn't take
> > > > > > // unreachable uses into account. We handle them here.
> > > > > > @@ -591,8 +599,27 @@ void llvm::deleteDeadLoop(Loop *L, Domin
> > > > > > "Unexpected user in reachable block");
> > > > > > U.set(Undef);
> > > > > > }
> > > > > > + auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I);
> > > > > > + if (!DVI)
> > > > > > + continue;
> > > > > > + auto Key = DeadDebugMap.find({DVI->getVariable(), DVI-
> > >getExpression()});
> > > > > > + if (Key != DeadDebugMap.end())
> > > > > > + continue;
> > > > > > + DeadDebugMap[{DVI->getVariable(), DVI-
> >getExpression()}]
> > = DVI;
> > > > > > + DeadDebugInst.push_back(DVI);
> > > > > > }
> > > > > >
> > > > > > + // After the loop has been deleted all the values defined
> and
> > modified
> > > > > > + // inside the loop are going to be unavailable.
> > > > > > + // Since debug values in the loop have been deleted,
> > inserting an undef
> > > > > > + // dbg.value truncates the range of any dbg.value before
> the
> > loop where the
> > > > > > + // loop used to be. This is particularly important for
> > constant values.
> > > > > > + DIBuilder DIB(*ExitBlock->getModule());
> > > > > > + for (auto *DVI : DeadDebugInst)
> > > > > > + DIB.insertDbgValueIntrinsic(
> > > > > > + UndefValue::get(DVI->getType()), DVI->getVariable(),
> > > > > > + DVI->getExpression(), DVI->getDebugLoc(), ExitBlock-
> > >getFirstNonPHI());
> > > > > > +
> > > > > > // Remove the block from the reference counting scheme,
> so
> > that we can
> > > > > > // delete it freely later.
> > > > > > for (auto *Block : L->blocks())
> > > > > >
> > > > > > Added: llvm/trunk/test/Transforms/LoopDeletion/diundef.ll
> > > > > > URL: http://llvm.org/viewvc/llvm-
> >
> project/llvm/trunk/test/Transforms/LoopDeletion/diundef.ll?rev=348988&v
> i
> > ew=auto
> > > > > >
> >
> =======================================================================
> =
> > ======
> > > > > > --- llvm/trunk/test/Transforms/LoopDeletion/diundef.ll
> (added)
> > > > > > +++ llvm/trunk/test/Transforms/LoopDeletion/diundef.ll Wed
> Dec
> > 12 15:32:35 2018
> > > > > > @@ -0,0 +1,75 @@
> > > > > > +; RUN: opt %s -loop-deletion -S | FileCheck %s
> > > > > > +
> > > > > > +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
> > > > > > +target triple = "x86_64-apple-macosx10.14.0"
> > > > > > +
> > > > > > + at a = common local_unnamed_addr global i32 0, align 4, !dbg
> !0
> > > > > > +
> > > > > > +define i32 @b() local_unnamed_addr !dbg !12 {
> > > > > > +entry:
> > > > > > + call void @llvm.dbg.value(metadata i32 0, metadata !16,
> > metadata !DIExpression()), !dbg !17
> > > > > > + br label %for.cond, !dbg !18
> > > > > > +
> > > > > > +for.cond: ; preds =
> > %for.cond, %entry
> > > > > > + %i.0 = phi i32 [ 0, %entry ], [ %inc, %for.cond ], !dbg
> !20
> > > > > > + call void @llvm.dbg.value(metadata i32 %i.0, metadata !16,
> > metadata !DIExpression()), !dbg !17
> > > > > > + %inc = add nuw nsw i32 %i.0, 1, !dbg !21
> > > > > > + call void @llvm.dbg.value(metadata i32 %inc, metadata !16,
> > metadata !DIExpression()), !dbg !17
> > > > > > + %exitcond = icmp ne i32 %inc, 3, !dbg !23
> > > > > > + br i1 %exitcond, label %for.cond, label %for.end, !dbg
> !24,
> > !llvm.loop !25
> > > > > > +
> > > > > > +; CHECK: call void @llvm.dbg.value(metadata void undef,
> > metadata !16, metadata !DIExpression()), !dbg !17
> > > > > > +; CHECK-NEXT: %call = tail call i32 {{.*}} @patatino()
> > > > > > +for.end: ; preds =
> > %for.cond
> > > > > > + %call = tail call i32 (...) @patatino() #3, !dbg !27
> > > > > > + %0 = load i32, i32* @a, align 4, !dbg !28
> > > > > > + ret i32 %0, !dbg !33
> > > > > > +}
> > > > > > +
> > > > > > +declare i32 @patatino(...) local_unnamed_addr
> > > > > > +
> > > > > > +define i32 @main() local_unnamed_addr !dbg !34 {
> > > > > > +entry:
> > > > > > + %call = call i32 @b(), !dbg !35
> > > > > > + ret i32 0, !dbg !36
> > > > > > +}
> > > > > > +
> > > > > > +declare void @llvm.dbg.value(metadata, metadata, metadata)
> > > > > > +
> > > > > > +!llvm.dbg.cu = !{!2}
> > > > > > +!llvm.module.flags = !{!7, !8, !9, !10}
> > > > > > +!llvm.ident = !{!11}
> > > > > > +
> > > > > > +!0 = !DIGlobalVariableExpression(var: !1, expr:
> > !DIExpression())
> > > > > > +!1 = distinct !DIGlobalVariable(name: "a", scope: !2, file:
> !3,
> > line: 1, type: !6, isLocal: false, isDefinition: true)
> > > > > > +!2 = distinct !DICompileUnit(language: DW_LANG_C99, file:
> !3,
> > producer: "clang version 8.0.0 ", isOptimized: true, runtimeVersion:
> 0,
> > emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: GNU)
> > > > > > +!3 = !DIFile(filename: "a.c", directory:
> > "/Users/davide/work/llvm-project-20170507/build-debug/bin")
> > > > > > +!4 = !{}
> > > > > > +!5 = !{!0}
> > > > > > +!6 = !DIBasicType(name: "int", size: 32, encoding:
> > DW_ATE_signed)
> > > > > > +!7 = !{i32 2, !"Dwarf Version", i32 4}
> > > > > > +!8 = !{i32 2, !"Debug Info Version", i32 3}
> > > > > > +!9 = !{i32 1, !"wchar_size", i32 4}
> > > > > > +!10 = !{i32 7, !"PIC Level", i32 2}
> > > > > > +!11 = !{!"clang version 8.0.0 "}
> > > > > > +!12 = distinct !DISubprogram(name: "b", scope: !3, file: !3,
> > line: 2, type: !13, scopeLine: 2, flags: DIFlagAllCallsDescribed,
> > spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2,
> > retainedNodes: !15)
> > > > > > +!13 = !DISubroutineType(types: !14)
> > > > > > +!14 = !{!6}
> > > > > > +!15 = !{!16}
> > > > > > +!16 = !DILocalVariable(name: "i", scope: !12, file: !3,
> line:
> > 3, type: !6)
> > > > > > +!17 = !DILocation(line: 3, column: 7, scope: !12)
> > > > > > +!18 = !DILocation(line: 4, column: 8, scope: !19)
> > > > > > +!19 = distinct !DILexicalBlock(scope: !12, file: !3, line:
> 4,
> > column: 3)
> > > > > > +!20 = !DILocation(line: 0, scope: !19)
> > > > > > +!21 = !DILocation(line: 4, column: 23, scope: !22)
> > > > > > +!22 = distinct !DILexicalBlock(scope: !19, file: !3, line:
> 4,
> > column: 3)
> > > > > > +!23 = !DILocation(line: 4, column: 17, scope: !22)
> > > > > > +!24 = !DILocation(line: 4, column: 3, scope: !19)
> > > > > > +!25 = distinct !{!25, !24, !26}
> > > > > > +!26 = !DILocation(line: 5, column: 5, scope: !19)
> > > > > > +!27 = !DILocation(line: 6, column: 3, scope: !12)
> > > > > > +!28 = !DILocation(line: 7, column: 10, scope: !12)
> > > > > > +!33 = !DILocation(line: 7, column: 3, scope: !12)
> > > > > > +!34 = distinct !DISubprogram(name: "main", scope: !3, file:
> !3,
> > line: 9, type: !13, scopeLine: 9, flags: DIFlagAllCallsDescribed,
> > spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2,
> > retainedNodes: !4)
> > > > > > +!35 = !DILocation(line: 9, column: 14, scope: !34)
> > > > > > +!36 = !DILocation(line: 9, column: 19, scope: !34)
> > > > > >
> > > > > >
> > > > > > _______________________________________________
> > > > > > llvm-commits mailing list
> > > > > > llvm-commits at lists.llvm.org
> > > > > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> > > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Davide
> > > >
> > > > "There are no solved problems; there are only problems that are
> more
> > > > or less solved" -- Henri Poincare
> > >
> > >
> > >
> > > --
> > > Davide
> > >
> > > "There are no solved problems; there are only problems that are
> more
> > > or less solved" -- Henri Poincare
> >
> >
> >
> > --
> > Davide
> >
> > "There are no solved problems; there are only problems that are more
> > or less solved" -- Henri Poincare
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list