[llvm] r277691 - Reinstate "[CloneFunction] Don't remove side effecting calls"
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 19 08:38:12 PDT 2016
I can reproduce the bug on trunk, but not on the 3.9 branch, so there
must be something more to it than just this commit.
On Fri, Aug 19, 2016 at 12:56 AM, Mikael Holmén
<mikael.holmen at ericsson.com> wrote:
> 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
>>
>
More information about the llvm-commits
mailing list