<div dir="ltr">FYI, this is assert failing on build bots: <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/13123">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/13123</a></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 30, 2014 at 5:13 PM, Gerolf Hoflehner <span dir="ltr"><<a href="mailto:ghoflehner@apple.com" target="_blank">ghoflehner@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ghoflehner<br>
Date: Tue Sep 30 19:13:22 2014<br>
New Revision: 218721<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=218721&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=218721&view=rev</a><br>
Log:<br>
[InstCombine] Optimize icmp-select-icmp<br>
<br>
In special cases select instructions can be eliminated by<br>
replacing them with a cheaper bitwise operation even when the<br>
select result is used outside its home block. The instances implemented<br>
are patterns like<br>
%x=icmp.eq<br>
%y=select %x,%r, null<br>
%z=icmp.eq|neq %y, null<br>
br %z,true, false<br>
==> %x=<a href="http://icmp.ne" target="_blank">icmp.ne</a><br>
%y=icmp.eq %r,null<br>
%z=or %x,%y<br>
br %z,true,false<br>
The optimization is integrated into the instruction<br>
combiner and performed only when all uses of the select result can<br>
be replaced by the select operand proper. For this dominator information<br>
is used and dominance is now a required analysis pass in the combiner.<br>
The optimization itself is iterative. The critical step is to replace the<br>
select result with the non-constant select operand. So the select becomes<br>
local and the combiner iteratively works out simpler code pattern and<br>
eventually eliminates the select.<br>
<br>
rdar://17853760<br>
<br>
<br>
Added:<br>
llvm/trunk/test/Transforms/InstCombine/select-cmp-br.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/InstCombine/InstCombine.h<br>
llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp<br>
llvm/trunk/test/Transforms/InstCombine/pr12338.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombine.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombine.h?rev=218721&r1=218720&r2=218721&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombine.h?rev=218721&r1=218720&r2=218721&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstCombine.h (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombine.h Tue Sep 30 19:13:22 2014<br>
@@ -14,6 +14,7 @@<br>
#include "llvm/Analysis/AssumptionTracker.h"<br>
#include "llvm/Analysis/TargetFolder.h"<br>
#include "llvm/Analysis/ValueTracking.h"<br>
+#include "llvm/IR/Dominators.h"<br>
#include "llvm/IR/IRBuilder.h"<br>
#include "llvm/IR/InstVisitor.h"<br>
#include "llvm/IR/IntrinsicInst.h"<br>
@@ -98,7 +99,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombin<br>
AssumptionTracker *AT;<br>
const DataLayout *DL;<br>
TargetLibraryInfo *TLI;<br>
- DominatorTree *DT; // not required<br>
+ DominatorTree *DT;<br>
bool MadeIRChange;<br>
LibCallSimplifier *Simplifier;<br>
bool MinimizeSize;<br>
@@ -113,7 +114,8 @@ public:<br>
BuilderTy *Builder;<br>
<br>
static char ID; // Pass identification, replacement for typeid<br>
- InstCombiner() : FunctionPass(ID), DL(nullptr), Builder(nullptr) {<br>
+ InstCombiner()<br>
+ : FunctionPass(ID), DL(nullptr), DT(nullptr), Builder(nullptr) {<br>
MinimizeSize = false;<br>
initializeInstCombinerPass(*PassRegistry::getPassRegistry());<br>
}<br>
@@ -242,6 +244,11 @@ public:<br>
<br>
// visitInstruction - Specify what to return for unhandled instructions...<br>
Instruction *visitInstruction(Instruction &I) { return nullptr; }<br>
+ bool dominatesAllUses(const Instruction *DI, const Instruction *UI,<br>
+ const BasicBlock *DB) const;<br>
+ bool replacedSelectWithOperand(SelectInst *SI, const ICmpInst *Icmp,<br>
+ const ConstantInt *CI1,<br>
+ const ConstantInt *CI2);<br>
<br>
private:<br>
bool ShouldChangeType(Type *From, Type *To) const;<br>
<br>
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=218721&r1=218720&r2=218721&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=218721&r1=218720&r2=218721&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Tue Sep 30 19:13:22 2014<br>
@@ -2429,6 +2429,127 @@ static bool swapMayExposeCSEOpportunitie<br>
return GlobalSwapBenefits > 0;<br>
}<br>
<br>
+/// \brief Check that one use is in the same block as the definition and all<br>
+/// other uses are in blocks dominated by a given block<br>
+///<br>
+/// \param DI Definition<br>
+/// \param UI Use<br>
+/// \param DB Block that must dominate all uses of \p DI outside<br>
+/// the parent block. Note there can be a use of \p DI in \p DB.<br>
+/// \return true when \p UI is the only use of \p DI in the parent block<br>
+/// and all other uses of \p DI are in blocks dominated by \p DB.<br>
+///<br>
+bool InstCombiner::dominatesAllUses(const Instruction *DI,<br>
+ const Instruction *UI,<br>
+ const BasicBlock *DB) const {<br>
+ assert(DI && DI->getParent() == UI->getParent() &&<br>
+ "definition and use must be in the same block");<br>
+ // DominatorTree available?<br>
+ if (!DT)<br>
+ return false;<br>
+ for (const User *U : DI->users()) {<br>
+ auto *Usr = cast<Instruction>(U);<br>
+ if (Usr != UI && !DT->dominates(DB, Usr->getParent()))<br>
+ return false;<br>
+ }<br>
+ return true;<br>
+}<br>
+<br>
+///<br>
+/// true when the instruction sequence within a block is select-cmp-br.<br>
+///<br>
+static bool isChainSelectCmpBranch(const SelectInst *SI) {<br>
+ const BasicBlock *BB = SI->getParent();<br>
+ if (!BB)<br>
+ return false;<br>
+ auto *BI = dyn_cast_or_null<BranchInst>(BB->getTerminator());<br>
+ if (!BI || BI->getNumSuccessors() != 2)<br>
+ return false;<br>
+ auto *IC = dyn_cast<ICmpInst>(BI->getCondition());<br>
+ if (!IC || (IC->getOperand(0) != SI && IC->getOperand(1) != SI))<br>
+ return false;<br>
+ return true;<br>
+}<br>
+<br>
+///<br>
+/// \brief True when a select result is replaced by one of its operands<br>
+/// in select-icmp sequence. This will eventually result in the elimination<br>
+/// of the select.<br>
+///<br>
+/// \param SI Select instruction<br>
+/// \param Icmp Compare instruction<br>
+/// \param CI1 'true' when first select operand is equal to RHSC of Icmp<br>
+/// \param CI2 'true' when second select operand is equal to RHSC of Icmp<br>
+///<br>
+/// Notes:<br>
+/// - The replacement is global and requires dominator information<br>
+/// - The caller is responsible for the actual replacement<br>
+///<br>
+/// Example:<br>
+///<br>
+/// entry:<br>
+/// %4 = select i1 %3, %C* %0, %C* null<br>
+/// %5 = icmp eq %C* %4, null<br>
+/// br i1 %5, label %9, label %7<br>
+/// ...<br>
+/// ; <label>:7 ; preds = %entry<br>
+/// %8 = getelementptr inbounds %C* %4, i64 0, i32 0<br>
+/// ...<br>
+///<br>
+/// can be transformed to<br>
+///<br>
+/// %5 = icmp eq %C* %0, null<br>
+/// %6 = select i1 %3, i1 %5, i1 true<br>
+/// br i1 %6, label %9, label %7<br>
+/// ...<br>
+/// ; <label>:7 ; preds = %entry<br>
+/// %8 = getelementptr inbounds %C* %0, i64 0, i32 0 // replace by %0!<br>
+///<br>
+/// Similar when the first operand of the select is a constant or/and<br>
+/// the compare is for not equal rather than equal.<br>
+///<br>
+/// FIXME: Currently the function considers equal compares only. It should be<br>
+/// possbile to extend it to not equal compares also.<br>
+///<br>
+bool InstCombiner::replacedSelectWithOperand(SelectInst *SI,<br>
+ const ICmpInst *Icmp,<br>
+ const ConstantInt *CI1,<br>
+ const ConstantInt *CI2) {<br>
+ if (isChainSelectCmpBranch(SI) && Icmp->isEquality()) {<br>
+ // Code sequence is select - icmp.[eq|ne] - br<br>
+ unsigned ReplaceWithOpd = 0;<br>
+ if (CI1 && !CI1->isZero())<br>
+ // The first constant operand of the select and the RHS of<br>
+ // the compare match, so try to substitute<br>
+ // the select results with its second operand<br>
+ // Example:<br>
+ // %4 = select i1 %3, %C* null, %C* %0<br>
+ // %5 = icmp eq %C* %4, null<br>
+ // ==> could replace select with second operand<br>
+ ReplaceWithOpd = 2;<br>
+ else if (CI2 && !CI2->isZero())<br>
+ // Similar when the second operand of the select is a constant<br>
+ // Example:<br>
+ // %4 = select i1 %3, %C* %0, %C* null<br>
+ // %5 = icmp eq %C* %4, null<br>
+ // ==> could replace select with first operand<br>
+ ReplaceWithOpd = 1;<br>
+ if (ReplaceWithOpd) {<br>
+ // Replace select with operand on else path for EQ compares.<br>
+ // Replace select with operand on then path for NE compares.<br>
+ BasicBlock *Succ =<br>
+ Icmp->getPredicate() == ICmpInst::ICMP_EQ<br>
+ ? SI->getParent()->getTerminator()->getSuccessor(1)<br>
+ : SI->getParent()->getTerminator()->getSuccessor(0);<br>
+ if (InstCombiner::dominatesAllUses(SI, Icmp, Succ)) {<br>
+ SI->replaceAllUsesWith(SI->getOperand(ReplaceWithOpd));<br>
+ return true;<br>
+ }<br>
+ }<br>
+ }<br>
+ return false;<br>
+}<br>
+<br>
Instruction *InstCombiner::visitICmpInst(ICmpInst &I) {<br>
bool Changed = false;<br>
Value *Op0 = I.getOperand(0), *Op1 = I.getOperand(1);<br>
@@ -2885,8 +3006,21 @@ Instruction *InstCombiner::visitICmpInst<br>
// fold to a constant (in which case the icmp is replaced with a select<br>
// which will usually simplify) or this is the only user of the<br>
// select (in which case we are trading a select+icmp for a simpler<br>
- // select+icmp).<br>
- if ((Op1 && Op2) || (LHSI->hasOneUse() && (Op1 || Op2))) {<br>
+ // select+icmp) or all uses of the select can be replaced based on<br>
+ // dominance information ("Global cases").<br>
+ bool Transform = false;<br>
+ if (Op1 && Op2)<br>
+ Transform = true;<br>
+ else if (Op1 || Op2) {<br>
+ if (LHSI->hasOneUse())<br>
+ Transform = true;<br>
+ else<br>
+ // Global cases<br>
+ Transform = replacedSelectWithOperand(<br>
+ cast<SelectInst>(LHSI), &I, dyn_cast_or_null<ConstantInt>(Op1),<br>
+ dyn_cast_or_null<ConstantInt>(Op2));<br>
+ }<br>
+ if (Transform) {<br>
if (!Op1)<br>
Op1 = Builder->CreateICmp(I.getPredicate(), LHSI->getOperand(1),<br>
RHSC, I.getName());<br>
<br>
Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=218721&r1=218720&r2=218721&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=218721&r1=218720&r2=218721&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Tue Sep 30 19:13:22 2014<br>
@@ -90,6 +90,7 @@ INITIALIZE_PASS_BEGIN(InstCombiner, "ins<br>
"Combine redundant instructions", false, false)<br>
INITIALIZE_PASS_DEPENDENCY(AssumptionTracker)<br>
INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfo)<br>
+INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)<br>
INITIALIZE_PASS_END(InstCombiner, "instcombine",<br>
"Combine redundant instructions", false, false)<br>
<br>
@@ -97,6 +98,8 @@ void InstCombiner::getAnalysisUsage(Anal<br>
AU.setPreservesCFG();<br>
AU.addRequired<AssumptionTracker>();<br>
AU.addRequired<TargetLibraryInfo>();<br>
+ AU.addRequired<DominatorTreeWrapperPass>();<br>
+ AU.addPreserved<DominatorTreeWrapperPass>();<br>
}<br>
<br>
<br>
@@ -2933,12 +2936,9 @@ bool InstCombiner::runOnFunction(Functio<br>
AT = &getAnalysis<AssumptionTracker>();<br>
DataLayoutPass *DLP = getAnalysisIfAvailable<DataLayoutPass>();<br>
DL = DLP ? &DLP->getDataLayout() : nullptr;<br>
+ DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();<br>
TLI = &getAnalysis<TargetLibraryInfo>();<br>
<br>
- DominatorTreeWrapperPass *DTWP =<br>
- getAnalysisIfAvailable<DominatorTreeWrapperPass>();<br>
- DT = DTWP ? &DTWP->getDomTree() : nullptr;<br>
-<br>
// Minimizing size?<br>
MinimizeSize = F.getAttributes().hasAttribute(AttributeSet::FunctionIndex,<br>
Attribute::MinSize);<br>
<br>
Modified: llvm/trunk/test/Transforms/InstCombine/pr12338.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/pr12338.ll?rev=218721&r1=218720&r2=218721&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/pr12338.ll?rev=218721&r1=218720&r2=218721&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/InstCombine/pr12338.ll (original)<br>
+++ llvm/trunk/test/Transforms/InstCombine/pr12338.ll Tue Sep 30 19:13:22 2014<br>
@@ -2,11 +2,11 @@<br>
<br>
define void @entry() nounwind {<br>
entry:<br>
+; CHECK: br label %for.cond<br>
br label %for.cond<br>
<br>
for.cond:<br>
%local = phi <1 x i32> [ <i32 0>, %entry ], [ %phi2, %cond.end47 ]<br>
-; CHECK: sub <1 x i32> <i32 92>, %local<br>
%phi3 = sub <1 x i32> zeroinitializer, %local<br>
br label %cond.end<br>
<br>
<br>
Added: llvm/trunk/test/Transforms/InstCombine/select-cmp-br.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/select-cmp-br.ll?rev=218721&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/select-cmp-br.ll?rev=218721&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/InstCombine/select-cmp-br.ll (added)<br>
+++ llvm/trunk/test/Transforms/InstCombine/select-cmp-br.ll Tue Sep 30 19:13:22 2014<br>
@@ -0,0 +1,127 @@<br>
+; Replace a 'select' with 'or' in 'select - cmp [eq|ne] - br' sequence<br>
+; RUN: opt -instcombine -S < %s | FileCheck %s<br>
+<br>
+%C = type <{ %struct.S }><br>
+%struct.S = type { i64*, i32, i32 }<br>
+<br>
+declare void @bar(%struct.S *) #1<br>
+<br>
+define void @test1(%C*) {<br>
+entry:<br>
+ %1 = getelementptr inbounds %C* %0, i64 0, i32 0, i32 0<br>
+ %m = load i64** %1, align 8<br>
+ %2 = getelementptr inbounds %C* %0, i64 1, i32 0, i32 0<br>
+ %n = load i64** %2, align 8<br>
+ %3 = getelementptr inbounds i64* %m, i64 9<br>
+ %4 = bitcast i64* %3 to i64 (%C*)**<br>
+ %5 = load i64 (%C*)** %4, align 8<br>
+ %6 = icmp eq i64* %m, %n<br>
+ %7 = select i1 %6, %C* %0, %C* null<br>
+ %8 = icmp eq %C* %7, null<br>
+ br i1 %8, label %12, label %10<br>
+<br>
+; <label>:9 ; preds = %10, %12<br>
+ ret void<br>
+<br>
+; <label>:10 ; preds = %entry<br>
+ %11 = getelementptr inbounds %C* %7, i64 0, i32 0<br>
+ tail call void @bar(%struct.S* %11)<br>
+ br label %9<br>
+<br>
+; <label>:12 ; preds = %entry<br>
+ %13 = tail call i64 %5(%C* %0)<br>
+ br label %9<br>
+; CHECK-LABEL: @test1(<br>
+; CHECK-NOT: select<br>
+; CHECK: or<br>
+}<br>
+<br>
+define void @test2(%C*) {<br>
+entry:<br>
+ %1 = getelementptr inbounds %C* %0, i64 0, i32 0, i32 0<br>
+ %m = load i64** %1, align 8<br>
+ %2 = getelementptr inbounds %C* %0, i64 1, i32 0, i32 0<br>
+ %n = load i64** %2, align 8<br>
+ %3 = getelementptr inbounds i64* %m, i64 9<br>
+ %4 = bitcast i64* %3 to i64 (%C*)**<br>
+ %5 = load i64 (%C*)** %4, align 8<br>
+ %6 = icmp eq i64* %m, %n<br>
+ %7 = select i1 %6, %C* null, %C* %0<br>
+ %8 = icmp eq %C* %7, null<br>
+ br i1 %8, label %12, label %10<br>
+<br>
+; <label>:9 ; preds = %10, %12<br>
+ ret void<br>
+<br>
+; <label>:10 ; preds = %entry<br>
+ %11 = getelementptr inbounds %C* %7, i64 0, i32 0<br>
+ tail call void @bar(%struct.S* %11)<br>
+ br label %9<br>
+<br>
+; <label>:12 ; preds = %entry<br>
+ %13 = tail call i64 %5(%C* %0)<br>
+ br label %9<br>
+; CHECK-LABEL: @test2(<br>
+; CHECK-NOT: select<br>
+; CHECK: or<br>
+}<br>
+<br>
+define void @test3(%C*) {<br>
+entry:<br>
+ %1 = getelementptr inbounds %C* %0, i64 0, i32 0, i32 0<br>
+ %m = load i64** %1, align 8<br>
+ %2 = getelementptr inbounds %C* %0, i64 1, i32 0, i32 0<br>
+ %n = load i64** %2, align 8<br>
+ %3 = getelementptr inbounds i64* %m, i64 9<br>
+ %4 = bitcast i64* %3 to i64 (%C*)**<br>
+ %5 = load i64 (%C*)** %4, align 8<br>
+ %6 = icmp eq i64* %m, %n<br>
+ %7 = select i1 %6, %C* %0, %C* null<br>
+ %8 = icmp ne %C* %7, null<br>
+ br i1 %8, label %10, label %12<br>
+<br>
+; <label>:9 ; preds = %10, %12<br>
+ ret void<br>
+<br>
+; <label>:10 ; preds = %entry<br>
+ %11 = getelementptr inbounds %C* %7, i64 0, i32 0<br>
+ tail call void @bar(%struct.S* %11)<br>
+ br label %9<br>
+<br>
+; <label>:12 ; preds = %entry<br>
+ %13 = tail call i64 %5(%C* %0)<br>
+ br label %9<br>
+; CHECK-LABEL: @test3(<br>
+; CHECK-NOT: select<br>
+; CHECK: or<br>
+}<br>
+<br>
+define void @test4(%C*) {<br>
+entry:<br>
+ %1 = getelementptr inbounds %C* %0, i64 0, i32 0, i32 0<br>
+ %m = load i64** %1, align 8<br>
+ %2 = getelementptr inbounds %C* %0, i64 1, i32 0, i32 0<br>
+ %n = load i64** %2, align 8<br>
+ %3 = getelementptr inbounds i64* %m, i64 9<br>
+ %4 = bitcast i64* %3 to i64 (%C*)**<br>
+ %5 = load i64 (%C*)** %4, align 8<br>
+ %6 = icmp eq i64* %m, %n<br>
+ %7 = select i1 %6, %C* null, %C* %0<br>
+ %8 = icmp ne %C* %7, null<br>
+ br i1 %8, label %10, label %12<br>
+<br>
+; <label>:9 ; preds = %10, %12<br>
+ ret void<br>
+<br>
+; <label>:10 ; preds = %entry<br>
+ %11 = getelementptr inbounds %C* %7, i64 0, i32 0<br>
+ tail call void @bar(%struct.S* %11)<br>
+ br label %9<br>
+<br>
+; <label>:12 ; preds = %entry<br>
+ %13 = tail call i64 %5(%C* %0)<br>
+ br label %9<br>
+; CHECK-LABEL: @test4(<br>
+; CHECK-NOT: select<br>
+; CHECK: or<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>