[llvm] r277691 - Reinstate "[CloneFunction] Don't remove side effecting calls"
David Majnemer via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 19 09:47:04 PDT 2016
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.
>
Should be fixed with r279268.
>
> (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/libpthr
> ead.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/Transform
>>>>> s/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/Transfor
>>>>> ms/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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160819/6a78b4be/attachment.html>
More information about the llvm-commits
mailing list