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