[llvm] r277691 - Reinstate "[CloneFunction] Don't remove side effecting calls"
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 19 00:56:29 PDT 2016
Hi David and Hans,
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.
(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.)
opt -S -inline -functionattrs red.ll
gives
opt: ../lib/Analysis/CallGraph.cpp:205: void
llvm::CallGraphNode::removeCallEdgeFor(llvm::CallSite): Assertion `I !=
CalledFunctions.end() && "Cannot find callsite to remove!"' failed.
#0 0x0000000001831288 llvm::sys::PrintStackTrace(llvm::raw_ostream&)
(build-all/bin/opt+0x1831288)
#1 0x0000000001831e66 SignalHandler(int) (build-all/bin/opt+0x1831e66)
#2 0x00007f26a8925330 __restore_rt
(/lib/x86_64-linux-gnu/libpthread.so.0+0x10330)
#3 0x00007f26a7518c37 gsignal
/build/eglibc-oGUzwX/eglibc-2.19/signal/../nptl/sysdeps/unix/sysv/linux/raise.c:56:0
#4 0x00007f26a751c028 abort
/build/eglibc-oGUzwX/eglibc-2.19/stdlib/abort.c:91:0
#5 0x00007f26a7511bf6 __assert_fail_base
/build/eglibc-oGUzwX/eglibc-2.19/assert/assert.c:92:0
#6 0x00007f26a7511ca2 (/lib/x86_64-linux-gnu/libc.so.6+0x2fca2)
#7 0x0000000000f6a4de
llvm::CallGraphNode::removeCallEdgeFor(llvm::CallSite)
(build-all/bin/opt+0xf6a4de)
#8 0x000000000186d13a llvm::InlineFunction(llvm::CallSite,
llvm::InlineFunctionInfo&, llvm::AAResults*, bool)
(build-all/bin/opt+0x186d13a)
#9 0x00000000014b4750 inlineCallsImpl(llvm::CallGraphSCC&,
llvm::CallGraph&, std::function<llvm::AssumptionCache&
(llvm::Function&)>, llvm::ProfileSummaryInfo*, llvm::TargetLibraryInfo&,
bool, std::function<llvm::InlineCost (llvm::CallSite)>,
std::function<llvm::AAResults& (llvm::Function&)>,
llvm::ImportedFunctionsInliningStatistics&) (build-all/bin/opt+0x14b4750)
#10 0x00000000014b2248 llvm::Inliner::inlineCalls(llvm::CallGraphSCC&)
(build-all/bin/opt+0x14b2248)
#11 0x00000000014b1fd2 llvm::Inliner::runOnSCC(llvm::CallGraphSCC&)
(build-all/bin/opt+0x14b1fd2)
#12 0x0000000000f6d823 (anonymous
namespace)::CGPassManager::runOnModule(llvm::Module&)
(build-all/bin/opt+0xf6d823)
#13 0x000000000141ef25 llvm::legacy::PassManagerImpl::run(llvm::Module&)
(build-all/bin/opt+0x141ef25)
#14 0x0000000000667025 main (build-all/bin/opt+0x667025)
#15 0x00007f26a7503f45 __libc_start_main
/build/eglibc-oGUzwX/eglibc-2.19/csu/libc-start.c:321:0
#16 0x0000000000656675 _start (build-all/bin/opt+0x656675)
Stack dump:
0. Program arguments: build-all/bin/opt -S -inline -functionattrs
red.ll
1. Running pass 'CallGraph Pass Manager' on module 'red.ll'.
Abort
-debug printouts:
Inliner visiting SCC: f1: 0 call sites.
CGSCCPASSMGR: Refreshing SCC with 1 nodes:
Call graph node for function: 'f1'<<0x4d327d0>> #uses=2
CGSCCPASSMGR: SCC Refresh didn't change call graph.
Inliner visiting SCC: f2: 0 call sites.
CGSCCPASSMGR: Refreshing SCC with 1 nodes:
Call graph node for function: 'f2'<<0x4d32c60>> #uses=2
CGSCCPASSMGR: SCC Refresh didn't change call graph.
Inliner visiting SCC: f3: 0 call sites.
CGSCCPASSMGR: Refreshing SCC with 1 nodes:
Call graph node for function: 'f3'<<0x4d32d30>> #uses=2
CGSCCPASSMGR: SCC Refresh didn't change call graph.
Inliner visiting SCC: f4: 3 call sites.
Analyzing call of f3...
NumConstantArgs: 0
NumConstantOffsetPtrArgs: 0
NumAllocaArgs: 0
NumConstantPtrCmps: 0
NumConstantPtrDiffs: 0
NumInstructionsSimplified: 1
NumInstructions: 1
SROACostSavings: 0
SROACostSavingsLost: 0
ContainsNoDuplicateCall: 0
Cost: 0
Threshold: 337
Inlining: cost=0, thres=337, Call: call void @f3()
Analyzing call of f1...
NumConstantArgs: 1
NumConstantOffsetPtrArgs: 0
NumAllocaArgs: 0
NumConstantPtrCmps: 0
NumConstantPtrDiffs: 0
NumInstructionsSimplified: 5
NumInstructions: 6
SROACostSavings: 0
SROACostSavingsLost: 0
ContainsNoDuplicateCall: 0
Cost: -5
Threshold: 225
Inlining: cost=-5, thres=225, Call: call void @f1(i16 0, i16 %_tmp29)
Analyzing call of f2...
NumConstantArgs: 1
NumConstantOffsetPtrArgs: 0
NumAllocaArgs: 0
NumConstantPtrCmps: 0
NumConstantPtrDiffs: 0
NumInstructionsSimplified: 1
NumInstructions: 1
SROACostSavings: 0
SROACostSavingsLost: 0
ContainsNoDuplicateCall: 0
Cost: -5
Threshold: 337
Inlining: cost=-5, thres=337, Call: %_tmp29 = call i16 @f2(i16 1)
opt: ../lib/Analysis/CallGraph.cpp:205: void
llvm::CallGraphNode::removeCallEdgeFor(llvm::CallSite): Assertion `I !=
CalledFunctions.end() && "Cannot find callsite to remove!"' failed.
Regards,
Mikael
On 08/05/2016 12:52 AM, Hans Wennborg via llvm-commits wrote:
> Thanks! I grabbed r277773 as well just to be sure :-) Merged in r277781.
>
> Since this is close to ToT, please let me know if any problems come up
> that require follow-ups.
>
> Cheers,
> Hans
>
> On Thu, Aug 4, 2016 at 10:59 AM, David Majnemer
> <david.majnemer at gmail.com> wrote:
>> Yes along with r277693.
>>
>> On Thu, Aug 4, 2016 at 10:13 AM, Hans Wennborg <hans at chromium.org> wrote:
>>>
>>> Should we merge this to 3.9?
>>>
>>> On Wed, Aug 3, 2016 at 9:24 PM, David Majnemer via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>>> Author: majnemer
>>>> Date: Wed Aug 3 23:24:02 2016
>>>> New Revision: 277691
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=277691&view=rev
>>>> Log:
>>>> Reinstate "[CloneFunction] Don't remove side effecting calls"
>>>>
>>>> This reinstates r277611 + r277614 and reverts r277642. A cast_or_null
>>>> should have been a dyn_cast_or_null.
>>>>
>>>> Modified:
>>>> llvm/trunk/lib/Analysis/InstructionSimplify.cpp
>>>> llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
>>>> llvm/trunk/test/Transforms/Inline/inline_constprop.ll
>>>>
>>>> Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=277691&r1=277690&r2=277691&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
>>>> +++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Wed Aug 3 23:24:02
>>>> 2016
>>>> @@ -4356,7 +4356,8 @@ static bool replaceAndRecursivelySimplif
>>>>
>>>> // Gracefully handle edge cases where the instruction is not wired
>>>> into any
>>>> // parent block.
>>>> - if (I->getParent())
>>>> + if (I->getParent() && !I->isEHPad() && !isa<TerminatorInst>(I) &&
>>>> + !I->mayHaveSideEffects())
>>>> I->eraseFromParent();
>>>> } else {
>>>> Worklist.insert(I);
>>>> @@ -4384,7 +4385,8 @@ static bool replaceAndRecursivelySimplif
>>>>
>>>> // Gracefully handle edge cases where the instruction is not wired
>>>> into any
>>>> // parent block.
>>>> - if (I->getParent())
>>>> + if (I->getParent() && !I->isEHPad() && !isa<TerminatorInst>(I) &&
>>>> + !I->mayHaveSideEffects())
>>>> I->eraseFromParent();
>>>> }
>>>> return Simplified;
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp?rev=277691&r1=277690&r2=277691&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp Wed Aug 3
>>>> 23:24:02 2016
>>>> @@ -14,6 +14,7 @@
>>>>
>>>> //===----------------------------------------------------------------------===//
>>>>
>>>> #include "llvm/Transforms/Utils/Cloning.h"
>>>> +#include "llvm/ADT/SetVector.h"
>>>> #include "llvm/ADT/SmallVector.h"
>>>> #include "llvm/Analysis/ConstantFolding.h"
>>>> #include "llvm/Analysis/InstructionSimplify.h"
>>>> @@ -552,9 +553,39 @@ void llvm::CloneAndPruneIntoFromInst(Fun
>>>> // two PHINodes, the iteration over the old PHIs remains valid, and
>>>> the
>>>> // mapping will just map us to the new node (which may not even be a
>>>> PHI
>>>> // node).
>>>> + const DataLayout &DL = NewFunc->getParent()->getDataLayout();
>>>> + SmallSetVector<const Value *, 8> Worklist;
>>>> for (unsigned Idx = 0, Size = PHIToResolve.size(); Idx != Size;
>>>> ++Idx)
>>>> - if (PHINode *PN = dyn_cast<PHINode>(VMap[PHIToResolve[Idx]]))
>>>> - recursivelySimplifyInstruction(PN);
>>>> + if (isa<PHINode>(VMap[PHIToResolve[Idx]]))
>>>> + Worklist.insert(PHIToResolve[Idx]);
>>>> +
>>>> + // Note that we must test the size on each iteration, the worklist
>>>> can grow.
>>>> + for (unsigned Idx = 0; Idx != Worklist.size(); ++Idx) {
>>>> + const Value *OrigV = Worklist[Idx];
>>>> + auto *I = cast_or_null<Instruction>(VMap.lookup(OrigV));
>>>> + if (!I)
>>>> + continue;
>>>> +
>>>> + // See if this instruction simplifies.
>>>> + Value *SimpleV = SimplifyInstruction(I, DL);
>>>> + if (!SimpleV)
>>>> + continue;
>>>> +
>>>> + // Stash away all the uses of the old instruction so we can check
>>>> them for
>>>> + // recursive simplifications after a RAUW. This is cheaper than
>>>> checking all
>>>> + // uses of To on the recursive step in most cases.
>>>> + for (const User *U : OrigV->users())
>>>> + Worklist.insert(cast<Instruction>(U));
>>>> +
>>>> + // Replace the instruction with its simplified value.
>>>> + I->replaceAllUsesWith(SimpleV);
>>>> +
>>>> + // If the original instruction had no side effects, remove it.
>>>> + if (isInstructionTriviallyDead(I))
>>>> + I->eraseFromParent();
>>>> + else
>>>> + VMap[OrigV] = I;
>>>> + }
>>>>
>>>> // Now that the inlined function body has been fully constructed, go
>>>> through
>>>> // and zap unconditional fall-through branches. This happens all the
>>>> time when
>>>>
>>>> Modified: llvm/trunk/test/Transforms/Inline/inline_constprop.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline_constprop.ll?rev=277691&r1=277690&r2=277691&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/Inline/inline_constprop.ll (original)
>>>> +++ llvm/trunk/test/Transforms/Inline/inline_constprop.ll Wed Aug 3
>>>> 23:24:02 2016
>>>> @@ -279,3 +279,25 @@ return:
>>>> %retval.0 = phi i32* [ %b, %if.end3 ], [ %a, %if.then ]
>>>> ret i32* %retval.0
>>>> }
>>>> +
>>>> +declare i32 @PR28802.external(i32 returned %p1)
>>>> +
>>>> +define internal i32 @PR28802.callee() {
>>>> +entry:
>>>> + br label %cont
>>>> +
>>>> +cont:
>>>> + %0 = phi i32 [ 0, %entry ]
>>>> + %call = call i32 @PR28802.external(i32 %0)
>>>> + ret i32 %call
>>>> +}
>>>> +
>>>> +define i32 @PR28802() {
>>>> +entry:
>>>> + %call = call i32 @PR28802.callee()
>>>> + ret i32 %call
>>>> +}
>>>> +
>>>> +; CHECK-LABEL: define i32 @PR28802(
>>>> +; CHECK: call i32 @PR28802.external(i32 0)
>>>> +; CHECK: ret i32 0
>>>>
>>>>
>>>> _______________________________________________
>>>> 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
>
-------------- next part --------------
; ModuleID = 'bugpoint-reduced-simplified.bc'
define void @f1(i16 %p1.1.par, i16 %p2.2.par) {
br label %bb1
bb1: ; preds = %0
br i1 false, label %bb4, label %bb3
bb3: ; preds = %bb1
br label %bb4
bb4: ; preds = %bb3, %bb1
%tmp0.3.0 = phi i16 [ %p2.2.par, %bb3 ], [ 0, %bb1 ]
%tmp1.4.0 = add i16 %tmp0.3.0, %p1.1.par
ret void
}
define i16 @f2(i16 %p1.5.par) {
bb2:
ret i16 %p1.5.par
}
define void @f3() {
ret void
}
define void @f4() {
bb1:
call void @f3()
%_tmp29 = call i16 @f2(i16 1)
call void @f1(i16 0, i16 %_tmp29)
ret void
}
More information about the llvm-commits
mailing list