[llvm] r286236 - [JumpThreading] Unfold selects that depend on the same condition
Haicheng Wu via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 15 13:01:28 PST 2016
Hi Pablo,
Thank you for doing this.
I also take another look at your patch. I think you may only need to unfold
selects belonging to the same basic block. If they are in two different
blocks, my understanding is that current jumpthreading implementation does
not have the capability of optimizing the unfolded blocks without something
like https://reviews.llvm.org/D22779.
Best,
Haicheng Wu
Employee of Qualcomm Datacenter Technologies, Inc.
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.
From: Pablo Barrio [mailto:Pablo.Barrio at arm.com]
Sent: Tuesday, November 15, 2016 10:15 AM
To: mcrosier at codeaurora.org
Cc: haicheng at codeaurora.org; llvm-commits at lists.llvm.org
Subject: Re: [llvm] r286236 - [JumpThreading] Unfold selects that depend on
the same condition
Sure, I'll revert now.
_____
From: <mailto:mcrosier at codeaurora.org> mcrosier at codeaurora.org <
<mailto:mcrosier at codeaurora.org> mcrosier at codeaurora.org>
Sent: 15 November 2016 14:18:20
To: Pablo Barrio
Cc: <mailto:haicheng at codeaurora.org> haicheng at codeaurora.org;
<mailto:llvm-commits at lists.llvm.org> llvm-commits at lists.llvm.org
Subject: Re: [llvm] r286236 - [JumpThreading] Unfold selects that depend on
the same condition
Hi Pablo,
Thanks for taking a look at this. If you don't think you'll have a fix
in the very near future would you mind reverting your commit while you
investigate?
Chad
On 2016-11-15 05:44, Pablo Barrio via llvm-commits wrote:
> Thank you Hai, I will have a look at it now.
>
> Pablo
>
> -------------------------
>
> FROM: <mailto:haicheng at codeaurora.org> haicheng at codeaurora.org <
<mailto:haicheng at codeaurora.org> haicheng at codeaurora.org>
> SENT: 15 November 2016 03:24:37
> TO: Pablo Barrio
> CC: <mailto:llvm-commits at lists.llvm.org> llvm-commits at lists.llvm.org
> SUBJECT: RE: [llvm] r286236 - [JumpThreading] Unfold selects that
> depend on the same condition
>
> Hi Pablo,
>
> It seems that your jumpthreading patch r286236 miscompiles
> llvm-test-suite/MultiSource/Benchmarks/SciMark2-C/scimark2 when using
> O3+LTO
> and running on AArch64.
>
> Here is the flag I used to compile " -O3 -flto -fuse-ld=gold
> -mcpu=cortex-a57"
> I used normal input size and got seg fault. Small input size seems
> okay.
>
> Please let me know if you need more information.
>
> Haicheng Wu
> Employee of Qualcomm Datacenter Technologies, Inc.
> Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
> Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
> Code
> Aurora Forum, a Linux Foundation Collaborative Project.
>
> -----Original Message-----
> From: llvm-commits [ <mailto:llvm-commits-bounces at lists.llvm.org>
mailto:llvm-commits-bounces at lists.llvm.org] On
> Behalf Of
> Pablo Barrio via llvm-commits
> Sent: Tuesday, November 08, 2016 9:54 AM
> To: <mailto:llvm-commits at lists.llvm.org> llvm-commits at lists.llvm.org
> Subject: [llvm] r286236 - [JumpThreading] Unfold selects that depend
> on
> the
> same condition
>
> Author: pabbar01
> Date: Tue Nov 8 08:53:30 2016
> New Revision: 286236
>
> URL: <http://llvm.org/viewvc/llvm-project?rev=286236&view=rev>
http://llvm.org/viewvc/llvm-project?rev=286236&view=rev
> Log:
> [JumpThreading] Unfold selects that depend on the same condition
>
> Summary:
> These are good candidates for jump threading. This enables later opts
> (such
> as InstCombine) to combine instructions from the selects with
> instructions
> out of the selects. SimplifyCFG will fold the select again if
> unfolding
> wasn't worth it.
>
> Patch by James Molloy and Pablo Barrio.
>
> Reviewers: rengolin, haicheng, sebpop
>
> Subscribers: jojo, jmolloy, llvm-commits
>
> Differential Revision: <https://reviews.llvm.org/D26391>
https://reviews.llvm.org/D26391
>
> Added:
>
> llvm/trunk/test/Transforms/JumpThreading/unfold-selects-same-cond.ll
> Modified:
> llvm/trunk/include/llvm/Transforms/Scalar/JumpThreading.h
> llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
>
> Modified: llvm/trunk/include/llvm/Transforms/Scalar/JumpThreading.h
> URL:
>
<http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Scal
ar/JumpThreading.h?rev=286236&r1=286235&r2=286236&view=diff>
http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Scala
r/JumpThreading.h?rev=286236&r1=286235&r2=286236&view=diff
>
============================================================================
==
> --- llvm/trunk/include/llvm/Transforms/Scalar/JumpThreading.h
> (original)
> +++ llvm/trunk/include/llvm/Transforms/Scalar/JumpThreading.h Tue Nov
> 8
> +++ 08:53:30 2016
> @@ -129,6 +129,8 @@ private:
> BasicBlock *NewBB, BasicBlock
> *SuccBB);
> /// Check if the block has profile metadata for its outgoing
> edges.
> bool doesBlockHaveProfileData(BasicBlock *BB);
> + SelectInst *getSelectFedByPhi(PHINode *PN); void
> + expandSelect(SelectInst *SI);
> };
>
> } // end namespace llvm
>
> Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
> URL:
>
<http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpTh
reading.cpp?rev=286236&r1=286235&r2=286236&view=diff>
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThr
eading.cpp?rev=286236&r1=286235&r2=286236&view=diff
>
============================================================================
==
> --- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Tue Nov 8
> +++ 08:53:30 2016
> @@ -1963,61 +1963,100 @@ bool JumpThreadingPass::TryToUnfoldSelec
> return false;
> }
>
> -/// TryToUnfoldSelectInCurrBB - Look for PHI/Select in the same BB of
>
> the
> form
> +/// GetSelectFedByPhi - Look for PHI/Select in the same BB of the
> form
> /// bb:
> /// %p = phi [false, %bb1], [true, %bb2], [false, %bb3], [true,
> %bb4],
> ...
> /// %s = select p, trueval, falseval
> ///
> -/// And expand the select into a branch structure. This later enables
> +/// And return the select. Unfolding it into a branch structure later
> +enables
> /// jump-threading over bb in this pass.
> ///
> -/// Using the similar approach of SimplifyCFG::FoldCondBranchOnPHI(),
> unfold -/// select if the associated PHI has at least one constant.
> If
> the
> unfolded -/// select is not jump-threaded, it will be folded again in
> the
> later -/// optimizations.
> +/// Using the similar approach of SimplifyCFG::FoldCondBranchOnPHI(),
> +return /// select if the associated PHI has at least one constant.
> +SelectInst *JumpThreadingPass::getSelectFedByPhi(PHINode *PN) {
> +
> + unsigned NumPHIValues = PN->getNumIncomingValues(); if
> (NumPHIValues
> + == 0 || !PN->hasOneUse())
> + return nullptr;
> +
> + SelectInst *SI = dyn_cast<SelectInst>(PN->user_back());
> + BasicBlock *BB = PN->getParent();
> + if (!SI || SI->getParent() != BB)
> + return nullptr;
> +
> + Value *Cond = SI->getCondition();
> + if (!Cond || Cond != PN || !Cond->getType()->isIntegerTy(1))
> + return nullptr;
> +
> + for (unsigned i = 0; i != NumPHIValues; ++i) {
> + if (PN->getIncomingBlock(i) == BB)
> + return nullptr;
> + if (isa<ConstantInt>(PN->getIncomingValue(i)))
> + return SI;
> + }
> +
> + return nullptr;
> +}
> +
> +/// ExpandSelect - Expand a select into an if-then-else construct.
> +void JumpThreadingPass::expandSelect(SelectInst *SI) {
> +
> + BasicBlock *BB = SI->getParent();
> + TerminatorInst *Term =
> + SplitBlockAndInsertIfThen(SI->getCondition(), SI, false);
> + PHINode *NewPN = PHINode::Create(SI->getType(), 2, "", SI);
> + NewPN->addIncoming(SI->getTrueValue(), Term->getParent());
> + NewPN->addIncoming(SI->getFalseValue(), BB);
> + SI->replaceAllUsesWith(NewPN);
> + SI->eraseFromParent();
> +}
> +
> +/// TryToUnfoldSelectInCurrBB - Unfold selects that could be
> +jump-threaded were /// they if-then-elses. If the unfolded selects
> are
> +not jump-threaded, it will /// be folded again in the later
> optimizations.
> bool JumpThreadingPass::TryToUnfoldSelectInCurrBB(BasicBlock *BB) {
> +
> // If threading this would thread across a loop header, don't
> thread
> the
> edge.
> // See the comments above FindLoopHeaders for justifications and
> caveats.
> if (LoopHeaders.count(BB))
> return false;
>
> - // Look for a Phi/Select pair in the same basic block. The Phi
> feeds
> the
> - // condition of the Select and at least one of the incoming values
> is
> a
> - // constant.
> - for (BasicBlock::iterator BI = BB->begin();
> - PHINode *PN = dyn_cast<PHINode>(BI); ++BI) {
> - unsigned NumPHIValues = PN->getNumIncomingValues();
> - if (NumPHIValues == 0 || !PN->hasOneUse())
> - continue;
> + bool Changed = false;
> + for (auto &I : *BB) {
>
> - SelectInst *SI = dyn_cast<SelectInst>(PN->user_back());
> - if (!SI || SI->getParent() != BB)
> + // Look for a Phi/Select pair in the same basic block. The Phi
> feeds
> the
> + // condition of the Select and at least one of the incoming
> values
> is a
> + // constant.
> + PHINode *PN;
> + SelectInst *SI;
> + if ((PN = dyn_cast<PHINode>(&I)) && (SI = getSelectFedByPhi(PN)))
> {
> + expandSelect(SI);
> + Changed = true;
> continue;
> + }
>
> - Value *Cond = SI->getCondition();
> - if (!Cond || Cond != PN || !Cond->getType()->isIntegerTy(1))
> - continue;
> + if (I.getType()->isIntegerTy(1)) {
>
> - bool HasConst = false;
> - for (unsigned i = 0; i != NumPHIValues; ++i) {
> - if (PN->getIncomingBlock(i) == BB)
> - return false;
> - if (isa<ConstantInt>(PN->getIncomingValue(i)))
> - HasConst = true;
> - }
> + SmallVector<SelectInst *, 4> Selects;
>
> - if (HasConst) {
> - // Expand the select.
> - TerminatorInst *Term =
> - SplitBlockAndInsertIfThen(SI->getCondition(), SI, false);
> - PHINode *NewPN = PHINode::Create(SI->getType(), 2, "", SI);
> - NewPN->addIncoming(SI->getTrueValue(), Term->getParent());
> - NewPN->addIncoming(SI->getFalseValue(), BB);
> - SI->replaceAllUsesWith(NewPN);
> - SI->eraseFromParent();
> - return true;
> + // Look for scalar booleans used in selects as conditions. If
> there
> are
> + // several selects that use the same boolean, they are
> candidates
> for
> jump
> + // threading and therefore we should unfold them.
> + for (Value *U : I.users())
> + if (auto *SI = dyn_cast<SelectInst>(U))
> + Selects.push_back(SI);
> + if (Selects.size() <= 1)
> + continue;
> +
> + // Remove duplicates
> + std::sort(Selects.begin(), Selects.end());
> + auto NewEnd = std::unique(Selects.begin(), Selects.end());
> +
> + Changed = true;
> + for (auto SI = Selects.begin(); SI != NewEnd; ++SI)
> + expandSelect(*SI);
> }
> }
> -
> - return false;
> +
> + return Changed;
> }
>
> Added:
> llvm/trunk/test/Transforms/JumpThreading/unfold-selects-same-cond.ll
> URL:
>
<http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreadin
g/unfold-selects-same-cond.ll?rev=286236&view=auto>
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading
/unfold-selects-same-cond.ll?rev=286236&view=auto
>
============================================================================
==
> ---
> llvm/trunk/test/Transforms/JumpThreading/unfold-selects-same-cond.ll
> (added)
> +++
> llvm/trunk/test/Transforms/JumpThreading/unfold-selects-same-cond.ll
> +++ Tue Nov 8 08:53:30 2016
> @@ -0,0 +1,45 @@
> +; RUN: opt < %s -jump-threading -instcombine -simplifycfg -S |
> +FileCheck %s
> +
> +; The three selects are jump-threaded so that instcombine can
> optimize,
> +and ; simplifycfg should turn the result into a single select.
> +define i32 @f(i32 %a, i32 %b) {
> +; CHECK: select
> +; CHECK-NOT: select
> +entry:
> + %0 = and i32 %a, 1
> + %1 = and i32 %b, 1
> + %xor = xor i32 %1, %a
> + %shr32 = lshr i32 %a, 1
> + %cmp10 = icmp eq i32 %xor, 1
> + %2 = xor i32 %b, 12345
> + %b.addr.1 = select i1 %cmp10, i32 %2, i32 %b
> + %shr1633 = lshr i32 %b.addr.1, 1
> + %3 = or i32 %shr1633, 54321
> + %b.addr.2 = select i1 %cmp10, i32 %3, i32 %shr1633
> + %shr1634 = lshr i32 %b.addr.2, 2
> + %4 = or i32 %shr1634, 54320
> + %b.addr.3 = select i1 %cmp10, i32 %4, i32 %shr1634
> + ret i32 %b.addr.3
> +}
> +
> +; Case where the condition is not only used as condition but also as
> +the ; true or false value in at least one of the selects.
> +define i1 @g(i32 %a, i32 %b) {
> +; CHECK: select
> +; CHECK-NOT: select
> +entry:
> + %0 = and i32 %a, 1
> + %1 = and i32 %b, 1
> + %xor = xor i32 %1, %a
> + %shr32 = lshr i32 %a, 1
> + %cmp10 = icmp eq i32 %xor, 1
> + %2 = xor i32 %b, 12345
> + %b.addr.1 = select i1 %cmp10, i32 %2, i32 %b
> + %shr1633 = lshr i32 %b.addr.1, 1
> + %3 = or i32 %shr1633, 54321
> + %b.addr.2 = select i1 %cmp10, i32 %3, i32 %shr1633
> + %shr1634 = lshr i32 %b.addr.2, 2
> + %4 = icmp eq i32 %shr1634, 54320
> + %b.addr.3 = select i1 %cmp10, i1 %4, i1 %cmp10
> + ret i1 %b.addr.3
> +}
>
> _______________________________________________
> llvm-commits mailing list
> <mailto:llvm-commits at lists.llvm.org> llvm-commits at lists.llvm.org
> <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose
> the contents to any other person, use it for any purpose, or store or
> copy the information in any medium. Thank you.
> _______________________________________________
> llvm-commits mailing list
> <mailto:llvm-commits at lists.llvm.org> llvm-commits at lists.llvm.org
> <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended
recipient, please notify the sender immediately and do not disclose the
contents to any other person, use it for any purpose, or store or copy the
information in any medium. Thank you.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161115/6b152d38/attachment.html>
More information about the llvm-commits
mailing list