[llvm] r305245 - Inliner: Don't remove calls to readnone+nounwind (but not always_inline) functions in the AlwaysInliner

[Björn Pettersson A via llvm-commits]

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