<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 19, 2016 at 12:56 AM, Mikael Holmén <span dir="ltr"><<a href="mailto:mikael.holmen@ericsson.com" target="_blank">mikael.holmen@ericsson.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi David and Hans,<br>
<br>
We've seen a crash with this commit. The below is a failed assertion, but with a variant of the input I've also seen a segmentation violation.<br></blockquote><div><br></div><div>Should be fixed with r279268.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
(I don't have a reproducer for the SEGV when compiling for any in-tree targets but I can try to get one if it still crashes after the assertion problem is fixed.)<br>
<br>
opt -S -inline -functionattrs red.ll<br>
<br>
gives<br>
<br>
opt: ../lib/Analysis/CallGraph.cpp:<wbr>205: void llvm::CallGraphNode::removeCal<wbr>lEdgeFor(llvm::CallSite): Assertion `I != CalledFunctions.end() && "Cannot find callsite to remove!"' failed.<br>
#0 0x0000000001831288 llvm::sys::PrintStackTrace(llv<wbr>m::raw_ostream&) (build-all/bin/opt+0x1831288)<br>
#1 0x0000000001831e66 SignalHandler(int) (build-all/bin/opt+0x1831e66)<br>
#2 0x00007f26a8925330 __restore_rt (/lib/x86_64-linux-gnu/libpthr<wbr>ead.so.0+0x10330)<br>
#3 0x00007f26a7518c37 gsignal /build/eglibc-oGUzwX/eglibc-2.<wbr>19/signal/../nptl/sysdeps/unix<wbr>/sysv/linux/raise.c:56:0<br>
#4 0x00007f26a751c028 abort /build/eglibc-oGUzwX/eglibc-2.<wbr>19/stdlib/abort.c:91:0<br>
#5 0x00007f26a7511bf6 __assert_fail_base /build/eglibc-oGUzwX/eglibc-2.<wbr>19/assert/assert.c:92:0<br>
#6 0x00007f26a7511ca2 (/lib/x86_64-linux-gnu/libc.so<wbr>.6+0x2fca2)<br>
#7 0x0000000000f6a4de llvm::CallGraphNode::removeCal<wbr>lEdgeFor(llvm::CallSite) (build-all/bin/opt+0xf6a4de)<br>
#8 0x000000000186d13a llvm::InlineFunction(llvm::Cal<wbr>lSite, llvm::InlineFunctionInfo&, llvm::AAResults*, bool) (build-all/bin/opt+0x186d13a)<br>
#9 0x00000000014b4750 inlineCallsImpl(llvm::CallGrap<wbr>hSCC&, llvm::CallGraph&, std::function<llvm::Assumption<wbr>Cache& (llvm::Function&)>, llvm::ProfileSummaryInfo*, llvm::TargetLibraryInfo&, bool, std::function<llvm::InlineCost (llvm::CallSite)>, std::function<llvm::AAResults& (llvm::Function&)>, llvm::ImportedFunctionsInlinin<wbr>gStatistics&) (build-all/bin/opt+0x14b4750)<br>
#10 0x00000000014b2248 llvm::Inliner::inlineCalls(llv<wbr>m::CallGraphSCC&) (build-all/bin/opt+0x14b2248)<br>
#11 0x00000000014b1fd2 llvm::Inliner::runOnSCC(llvm::<wbr>CallGraphSCC&) (build-all/bin/opt+0x14b1fd2)<br>
#12 0x0000000000f6d823 (anonymous namespace)::CGPassManager::run<wbr>OnModule(llvm::Module&) (build-all/bin/opt+0xf6d823)<br>
#13 0x000000000141ef25 llvm::legacy::PassManagerImpl:<wbr>:run(llvm::Module&) (build-all/bin/opt+0x141ef25)<br>
#14 0x0000000000667025 main (build-all/bin/opt+0x667025)<br>
#15 0x00007f26a7503f45 __libc_start_main /build/eglibc-oGUzwX/eglibc-2.<wbr>19/csu/libc-start.c:321:0<br>
#16 0x0000000000656675 _start (build-all/bin/opt+0x656675)<br>
Stack dump:<br>
0. Program arguments: build-all/bin/opt -S -inline -functionattrs red.ll<br>
1. Running pass 'CallGraph Pass Manager' on module 'red.ll'.<br>
Abort<br>
<br>
-debug printouts:<br>
<br>
Inliner visiting SCC: f1: 0 call sites.<br>
CGSCCPASSMGR: Refreshing SCC with 1 nodes:<br>
Call graph node for function: 'f1'<<0x4d327d0>> #uses=2<br>
<br>
CGSCCPASSMGR: SCC Refresh didn't change call graph.<br>
Inliner visiting SCC: f2: 0 call sites.<br>
CGSCCPASSMGR: Refreshing SCC with 1 nodes:<br>
Call graph node for function: 'f2'<<0x4d32c60>> #uses=2<br>
<br>
CGSCCPASSMGR: SCC Refresh didn't change call graph.<br>
Inliner visiting SCC: f3: 0 call sites.<br>
CGSCCPASSMGR: Refreshing SCC with 1 nodes:<br>
Call graph node for function: 'f3'<<0x4d32d30>> #uses=2<br>
<br>
CGSCCPASSMGR: SCC Refresh didn't change call graph.<br>
Inliner visiting SCC: f4: 3 call sites.<br>
Analyzing call of f3...<br>
NumConstantArgs: 0<br>
NumConstantOffsetPtrArgs: 0<br>
NumAllocaArgs: 0<br>
NumConstantPtrCmps: 0<br>
NumConstantPtrDiffs: 0<br>
NumInstructionsSimplified: 1<br>
NumInstructions: 1<br>
SROACostSavings: 0<br>
SROACostSavingsLost: 0<br>
ContainsNoDuplicateCall: 0<br>
Cost: 0<br>
Threshold: 337<br>
Inlining: cost=0, thres=337, Call: call void @f3()<br>
Analyzing call of f1...<br>
NumConstantArgs: 1<br>
NumConstantOffsetPtrArgs: 0<br>
NumAllocaArgs: 0<br>
NumConstantPtrCmps: 0<br>
NumConstantPtrDiffs: 0<br>
NumInstructionsSimplified: 5<br>
NumInstructions: 6<br>
SROACostSavings: 0<br>
SROACostSavingsLost: 0<br>
ContainsNoDuplicateCall: 0<br>
Cost: -5<br>
Threshold: 225<br>
Inlining: cost=-5, thres=225, Call: call void @f1(i16 0, i16 %_tmp29)<br>
Analyzing call of f2...<br>
NumConstantArgs: 1<br>
NumConstantOffsetPtrArgs: 0<br>
NumAllocaArgs: 0<br>
NumConstantPtrCmps: 0<br>
NumConstantPtrDiffs: 0<br>
NumInstructionsSimplified: 1<br>
NumInstructions: 1<br>
SROACostSavings: 0<br>
SROACostSavingsLost: 0<br>
ContainsNoDuplicateCall: 0<br>
Cost: -5<br>
Threshold: 337<br>
Inlining: cost=-5, thres=337, Call: %_tmp29 = call i16 @f2(i16 1)<br>
opt: ../lib/Analysis/CallGraph.cpp:<wbr>205: void llvm::CallGraphNode::removeCal<wbr>lEdgeFor(llvm::CallSite): Assertion `I != CalledFunctions.end() && "Cannot find callsite to remove!"' failed.<br>
<br>
Regards,<br>
Mikael<div class=""><div class="h5"><br>
<br>
On 08/05/2016 12:52 AM, Hans Wennborg via llvm-commits wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Thanks! I grabbed r277773 as well just to be sure :-) Merged in r277781.<br>
<br>
Since this is close to ToT, please let me know if any problems come up<br>
that require follow-ups.<br>
<br>
Cheers,<br>
Hans<br>
<br>
On Thu, Aug 4, 2016 at 10:59 AM, David Majnemer<br>
<<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Yes along with r277693.<br>
<br>
On Thu, Aug 4, 2016 at 10:13 AM, Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Should we merge this to 3.9?<br>
<br>
On Wed, Aug 3, 2016 at 9:24 PM, David Majnemer via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Author: majnemer<br>
Date: Wed Aug 3 23:24:02 2016<br>
New Revision: 277691<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=277691&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=277691&view=rev</a><br>
Log:<br>
Reinstate "[CloneFunction] Don't remove side effecting calls"<br>
<br>
This reinstates r277611 + r277614 and reverts r277642. A cast_or_null<br>
should have been a dyn_cast_or_null.<br>
<br>
Modified:<br>
llvm/trunk/lib/Analysis/Instru<wbr>ctionSimplify.cpp<br>
llvm/trunk/lib/Transforms/Util<wbr>s/CloneFunction.cpp<br>
llvm/trunk/test/Transforms/Inl<wbr>ine/inline_constprop.ll<br>
<br>
Modified: llvm/trunk/lib/Analysis/Instru<wbr>ctionSimplify.cpp<br>
URL:<br>
<a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=277691&r1=277690&r2=277691&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Analysis/<wbr>InstructionSimplify.cpp?rev=<wbr>277691&r1=277690&r2=277691&<wbr>view=diff</a><br>
<br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Analysis/Instru<wbr>ctionSimplify.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/Instru<wbr>ctionSimplify.cpp Wed Aug 3 23:24:02<br>
2016<br>
@@ -4356,7 +4356,8 @@ static bool replaceAndRecursivelySimplif<br>
<br>
// Gracefully handle edge cases where the instruction is not wired<br>
into any<br>
// parent block.<br>
- if (I->getParent())<br>
+ if (I->getParent() && !I->isEHPad() && !isa<TerminatorInst>(I) &&<br>
+ !I->mayHaveSideEffects())<br>
I->eraseFromParent();<br>
} else {<br>
Worklist.insert(I);<br>
@@ -4384,7 +4385,8 @@ static bool replaceAndRecursivelySimplif<br>
<br>
// Gracefully handle edge cases where the instruction is not wired<br>
into any<br>
// parent block.<br>
- if (I->getParent())<br>
+ if (I->getParent() && !I->isEHPad() && !isa<TerminatorInst>(I) &&<br>
+ !I->mayHaveSideEffects())<br>
I->eraseFromParent();<br>
}<br>
return Simplified;<br>
<br>
Modified: llvm/trunk/lib/Transforms/Util<wbr>s/CloneFunction.cpp<br>
URL:<br>
<a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp?rev=277691&r1=277690&r2=277691&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Transform<wbr>s/Utils/CloneFunction.cpp?rev=<wbr>277691&r1=277690&r2=277691&<wbr>view=diff</a><br>
<br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Transforms/Util<wbr>s/CloneFunction.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Util<wbr>s/CloneFunction.cpp Wed Aug 3<br>
23:24:02 2016<br>
@@ -14,6 +14,7 @@<br>
<br>
//===-------------------------<wbr>------------------------------<wbr>---------------===//<br>
<br>
#include "llvm/Transforms/Utils/Cloning<wbr>.h"<br>
+#include "llvm/ADT/SetVector.h"<br>
#include "llvm/ADT/SmallVector.h"<br>
#include "llvm/Analysis/ConstantFolding<wbr>.h"<br>
#include "llvm/Analysis/InstructionSimp<wbr>lify.h"<br>
@@ -552,9 +553,39 @@ void llvm::CloneAndPruneIntoFromIns<wbr>t(Fun<br>
// two PHINodes, the iteration over the old PHIs remains valid, and<br>
the<br>
// mapping will just map us to the new node (which may not even be a<br>
PHI<br>
// node).<br>
+ const DataLayout &DL = NewFunc->getParent()->getDataL<wbr>ayout();<br>
+ SmallSetVector<const Value *, 8> Worklist;<br>
for (unsigned Idx = 0, Size = PHIToResolve.size(); Idx != Size;<br>
++Idx)<br>
- if (PHINode *PN = dyn_cast<PHINode>(VMap[PHIToRe<wbr>solve[Idx]]))<br>
- recursivelySimplifyInstruction<wbr>(PN);<br>
+ if (isa<PHINode>(VMap[PHIToResolv<wbr>e[Idx]]))<br>
+ Worklist.insert(PHIToResolve[I<wbr>dx]);<br>
+<br>
+ // Note that we must test the size on each iteration, the worklist<br>
can grow.<br>
+ for (unsigned Idx = 0; Idx != Worklist.size(); ++Idx) {<br>
+ const Value *OrigV = Worklist[Idx];<br>
+ auto *I = cast_or_null<Instruction>(VMap<wbr>.lookup(OrigV));<br>
+ if (!I)<br>
+ continue;<br>
+<br>
+ // See if this instruction simplifies.<br>
+ Value *SimpleV = SimplifyInstruction(I, DL);<br>
+ if (!SimpleV)<br>
+ continue;<br>
+<br>
+ // Stash away all the uses of the old instruction so we can check<br>
them for<br>
+ // recursive simplifications after a RAUW. This is cheaper than<br>
checking all<br>
+ // uses of To on the recursive step in most cases.<br>
+ for (const User *U : OrigV->users())<br>
+ Worklist.insert(cast<Instructi<wbr>on>(U));<br>
+<br>
+ // Replace the instruction with its simplified value.<br>
+ I->replaceAllUsesWith(SimpleV)<wbr>;<br>
+<br>
+ // If the original instruction had no side effects, remove it.<br>
+ if (isInstructionTriviallyDead(I)<wbr>)<br>
+ I->eraseFromParent();<br>
+ else<br>
+ VMap[OrigV] = I;<br>
+ }<br>
<br>
// Now that the inlined function body has been fully constructed, go<br>
through<br>
// and zap unconditional fall-through branches. This happens all the<br>
time when<br>
<br>
Modified: llvm/trunk/test/Transforms/Inl<wbr>ine/inline_constprop.ll<br>
URL:<br>
<a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline_constprop.ll?rev=277691&r1=277690&r2=277691&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/Transfor<wbr>ms/Inline/inline_constprop.ll?<wbr>rev=277691&r1=277690&r2=<wbr>277691&view=diff</a><br>
<br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Transforms/Inl<wbr>ine/inline_constprop.ll (original)<br>
+++ llvm/trunk/test/Transforms/Inl<wbr>ine/inline_constprop.ll Wed Aug 3<br>
23:24:02 2016<br>
@@ -279,3 +279,25 @@ return:<br>
%retval.0 = phi i32* [ %b, %if.end3 ], [ %a, %if.then ]<br>
ret i32* %retval.0<br>
}<br>
+<br>
+declare i32 @PR28802.external(i32 returned %p1)<br>
+<br>
+define internal i32 @PR28802.callee() {<br>
+entry:<br>
+ br label %cont<br>
+<br>
+cont:<br>
+ %0 = phi i32 [ 0, %entry ]<br>
+ %call = call i32 @PR28802.external(i32 %0)<br>
+ ret i32 %call<br>
+}<br>
+<br>
+define i32 @PR28802() {<br>
+entry:<br>
+ %call = call i32 @PR28802.callee()<br>
+ ret i32 %call<br>
+}<br>
+<br>
+; CHECK-LABEL: define i32 @PR28802(<br>
+; CHECK: call i32 @PR28802.external(i32 0)<br>
+; CHECK: ret i32 0<br>
<br>
<br>
______________________________<wbr>_________________<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/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></blockquote>
<br>
<br>
</blockquote>
______________________________<wbr>_________________<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/<wbr>mailman/listinfo/llvm-commits</a><br>
<br>
</blockquote>
</div></div></blockquote></div><br></div></div>