[llvm] r305245 - Inliner: Don't remove calls to readnone+nounwind (but not always_inline) functions in the AlwaysInliner
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 15 17:16:44 PDT 2017
Looks like this has exposed a bug in instcombine where instcombine will
remove a trivially dead call which can invalidate the call graph (yet
instcombine claims to preserve the call graph).
A trivial change to avoid removing such calls seems to be tripping up some
things for me... I think it's because a bunch of instcombine at least
relies on removing trivially dead calls to intrinsics, perhaps.
hrm...
On Thu, Jun 15, 2017 at 6:40 AM Björn Pettersson A <
bjorn.a.pettersson at ericsson.com> wrote:
> A smaller reproducer:
>
>
> ;-------------------------------------------------------------------------------
> ; opt -S -functionattrs -inline -instcombine -o -
>
> ; Function Attrs: noinline norecurse nounwind readnone
> define void @getPageAddr() #0 {
> entry:
> %call = call i32 @cmAddr()
> ret void
> }
>
> ; Function Attrs: noinline norecurse nounwind readnone
> define internal i32 @cmAddr() #0 {
> entry:
> ret i32 5
> }
>
> attributes #0 = { noinline norecurse nounwind readnone }
>
> ;-------------------------------------------------------------------------------
>
>
> Note that if I execute this using
> opt -S -functionattrs -inline -print-callgraph -instcombine -o -
> instead of
> opt -S -functionattrs -inline -instcombine -o -
> the assert won't trigger (although, cmAddr is not removed and I think it
> asserts when trying to remove that function).
>
> Regards,
> Björn
>
> > -----Original Message-----
> > From: Mikael Holmén
> > Sent: den 15 juni 2017 13:02
> > To: David Blaikie <dblaikie at gmail.com>; llvm-commits at lists.llvm.org
> > Cc: Björn Pettersson A <bjorn.a.pettersson at ericsson.com>
> > Subject: Re: [llvm] r305245 - Inliner: Don't remove calls to
> > readnone+nounwind (but not always_inline) functions in the AlwaysInliner
> >
> > Hi David,
> >
> > With this commit, opt crashes when doing:
> >
> > opt -S -functionattrs -inline -instcombine -o - callgraph.ll
> >
> > opt: ../include/llvm/Analysis/CallGraph.h:174:
> > llvm::CallGraphNode::~CallGraphNode(): Assertion `NumReferences == 0 &&
> > "Node deleted while references remain"' failed.
> >
> > Regards,
> > Mikael
> >
> > On 06/13/2017 01:01 AM, David Blaikie via llvm-commits wrote:
> > > Author: dblaikie
> > > Date: Mon Jun 12 18:01:17 2017
> > > New Revision: 305245
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=305245&view=rev
> > > Log:
> > > Inliner: Don't remove calls to readnone+nounwind (but not
> always_inline)
> > functions in the AlwaysInliner
> > >
> > > Modified:
> > > llvm/trunk/lib/Transforms/IPO/Inliner.cpp
> > > llvm/trunk/test/Transforms/Inline/always-inline.ll
> > >
> > > Modified: llvm/trunk/lib/Transforms/IPO/Inliner.cpp
> > > URL: http://llvm.org/viewvc/llvm-
> > project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=305245&r1=305244&
> > r2=305245&view=diff
> > >
> > ==========================================================
> > ====================
> > > --- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)
> > > +++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Mon Jun 12 18:01:17 2017
> > > @@ -523,6 +523,16 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG
> > > if (!Callee || Callee->isDeclaration())
> > > continue;
> > >
> > > + // FIXME for new PM: because of the old PM we currently generate
> > ORE and
> > > + // in turn BFI on demand. With the new PM, the ORE dependency
> > should
> > > + // just become a regular analysis dependency.
> > > + OptimizationRemarkEmitter ORE(Caller);
> > > +
> > > + // If the policy determines that we should inline this function,
> > > + // delete the call instead.
> > > + if (!shouldInline(CS, GetInlineCost, ORE))
> > > + continue;
> > > +
> > > // If this call site is dead and it is to a readonly function,
> we should
> > > // just delete the call instead of trying to inline it,
> regardless of
> > > // size. This happens because IPSCCP propagates the result
> out of the
> > > @@ -548,15 +558,6 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG
> > > // Get DebugLoc to report. CS will be invalid after Inliner.
> > > DebugLoc DLoc = CS.getInstruction()->getDebugLoc();
> > > BasicBlock *Block = CS.getParent();
> > > - // FIXME for new PM: because of the old PM we currently
> generate
> > ORE and
> > > - // in turn BFI on demand. With the new PM, the ORE dependency
> > should
> > > - // just become a regular analysis dependency.
> > > - OptimizationRemarkEmitter ORE(Caller);
> > > -
> > > - // If the policy determines that we should inline this
> function,
> > > - // try to do so.
> > > - if (!shouldInline(CS, GetInlineCost, ORE))
> > > - continue;
> > >
> > > // Attempt to inline the function.
> > > using namespace ore;
> > >
> > > Modified: llvm/trunk/test/Transforms/Inline/always-inline.ll
> > > URL: http://llvm.org/viewvc/llvm-
> > project/llvm/trunk/test/Transforms/Inline/always-
> > inline.ll?rev=305245&r1=305244&r2=305245&view=diff
> > >
> > ==========================================================
> > ====================
> > > --- llvm/trunk/test/Transforms/Inline/always-inline.ll (original)
> > > +++ llvm/trunk/test/Transforms/Inline/always-inline.ll Mon Jun 12
> 18:01:17
> > 2017
> > > @@ -305,3 +305,14 @@ entry:
> > > ret void
> > > ; CHECK: ret void
> > > }
> > > +
> > > +define void @inner14() readnone nounwind {
> > > +; CHECK: define void @inner14
> > > + ret void
> > > +}
> > > +
> > > +define void @outer14() {
> > > +; CHECK: call void @inner14
> > > + call void @inner14()
> > > + ret void
> > > +}
> > >
> > >
> > > _______________________________________________
> > > 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/20170616/6f260946/attachment.html>
More information about the llvm-commits
mailing list