<div dir="ltr">I will think about, the problem now is that patch got reverted because it got stuck on GVN on one build, so I firstly have to fix it.<div><br></div><div>Piotr</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 28, 2015 at 4:57 PM, Philip Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
Right, but you could write a new routine which does something like
the following:<br>
<br>
replaceDominatedUsesWith:<br>
for each use in uses:<br>
if use in same BB: set BB scan flag<br>
if use in dominated BB: update<br>
if bb scan flag:<br>
from start to BB->end():<br>
replace use of from with to<br>
<br>
The second loop should run exactly once ever. The first loop is as
fast as the existing use of replaceDominatesUsesWith. <br>
<br>
That seems simpler than the current implementation.<br>
<br>
For the record, feel free to ignore the comment. I'm just making an
observation, not demanding a change.<span class="HOEnZb"><font color="#888888"><br>
<br>
Philip</font></span><div><div class="h5"><br>
<br>
<br>
<br>
<br>
<div>On 08/28/2015 04:44 PM, Piotr Padlewski
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">replaceDominatedUsesWith works across different
BBs, and I wanted to have something that will replace all the
non const variables with const ones after hiting @llvm.assume.
<div><br>
</div>
<div>Piotr</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Fri, Aug 28, 2015 at 9:54 AM, Philip
Reames <span dir="ltr"><<a href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Piotr,<br>
<br>
I'm a bit confused by the need for the ReplaceWithConstMap
variable in this patch. Wouldn't simply calling
replaceDominatedUsesWith be sufficient? Is there a reason
to prefer the BB local variable?<span><font color="#888888"><br>
<br>
Philip</font></span>
<div>
<div><br>
<br>
<br>
On 08/27/2015 06:01 PM, Piotr Padlewski via llvm-commits
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: prazek<br>
Date: Thu Aug 27 20:01:57 2015<br>
New Revision: 246243<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=246243&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=246243&view=rev</a><br>
Log:<br>
Constant propagation after hiting llvm.assume<br>
<br>
After hitting @llvm.assume(X) we can:<br>
- propagate equality that X == true<br>
- if X is icmp/fcmp (with eq operation), and one of
operand<br>
is constant we can change all variables with
constants in the same BasicBlock<br>
<br>
<a href="http://reviews.llvm.org/D11918" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11918</a><br>
<br>
Added:<br>
llvm/trunk/test/Transforms/GVN/assume-equal.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/Scalar/GVN.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=246243&r1=246242&r2=246243&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=246243&r1=246242&r2=246243&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp
(original)<br>
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Thu Aug
27 20:01:57 2015<br>
@@ -608,6 +608,10 @@ namespace {<br>
DenseMap<uint32_t, LeaderTableEntry>
LeaderTable;<br>
BumpPtrAllocator TableAllocator;<br>
+ // Block-local map of equivalent values to
their leader, does not<br>
+ // propagate to any successors. Entries added
mid-block are applied<br>
+ // to the remaining instructions in the block.<br>
+ SmallMapVector<llvm::Value *, llvm::Constant
*, 4> ReplaceWithConstMap;<br>
SmallVector<Instruction*, 8>
InstrsToErase;<br>
typedef SmallVector<NonLocalDepResult,
64> LoadDepVect;<br>
@@ -699,6 +703,7 @@ namespace {<br>
// Helper functions of redundant load
elimination<br>
bool processLoad(LoadInst *L);<br>
bool processNonLocalLoad(LoadInst *L);<br>
+ bool processAssumeIntrinsic(IntrinsicInst *II);<br>
void AnalyzeLoadAvailability(LoadInst *LI,
LoadDepVect &Deps,<br>
AvailValInBlkVect
&ValuesPerBlock,<br>
UnavailBlkVect
&UnavailableBlocks);<br>
@@ -719,6 +724,7 @@ namespace {<br>
void verifyRemoved(const Instruction *I) const;<br>
bool splitCriticalEdges();<br>
BasicBlock *splitCriticalEdges(BasicBlock *Pred,
BasicBlock *Succ);<br>
+ bool replaceOperandsWithConsts(Instruction *I)
const;<br>
bool propagateEquality(Value *LHS, Value *RHS,
const BasicBlockEdge &Root);<br>
bool processFoldableCondBr(BranchInst *BI);<br>
void addDeadBlock(BasicBlock *BB);<br>
@@ -1760,6 +1766,38 @@ bool
GVN::processNonLocalLoad(LoadInst *<br>
return PerformLoadPRE(LI, ValuesPerBlock,
UnavailableBlocks);<br>
}<br>
+bool GVN::processAssumeIntrinsic(IntrinsicInst
*IntrinsicI) {<br>
+ assert(IntrinsicI->getIntrinsicID() ==
Intrinsic::assume &&<br>
+ "This function can only be called with
llvm.assume intrinsic");<br>
+ Value *V = IntrinsicI->getArgOperand(0);<br>
+ Constant *True =
ConstantInt::getTrue(V->getContext());<br>
+ bool Changed = false;<br>
+ for (BasicBlock *Successor :
successors(IntrinsicI->getParent())) {<br>
+ BasicBlockEdge Edge(IntrinsicI->getParent(),
Successor);<br>
+<br>
+ // This property is only true in dominated
successors, propagateEquality<br>
+ // will check dominance for us.<br>
+ Changed |= propagateEquality(V, True, Edge);<br>
+ }<br>
+<br>
+ if (auto *CmpI = dyn_cast<CmpInst>(V)) {<br>
+ if (CmpI->getPredicate() ==
CmpInst::Predicate::ICMP_EQ ||<br>
+ CmpI->getPredicate() ==
CmpInst::Predicate::FCMP_OEQ ||<br>
+ (CmpI->getPredicate() ==
CmpInst::Predicate::FCMP_UEQ &&<br>
+ CmpI->getFastMathFlags().noNaNs())) {<br>
+ Value *CmpLHS = CmpI->getOperand(0);<br>
+ Value *CmpRHS = CmpI->getOperand(1);<br>
+ if (isa<Constant>(CmpLHS))<br>
+ std::swap(CmpLHS, CmpRHS);<br>
+ auto *RHSConst =
dyn_cast<Constant>(CmpRHS);<br>
+<br>
+ // If only one operand is constant.<br>
+ if (RHSConst != nullptr &&
!isa<Constant>(CmpLHS))<br>
+ ReplaceWithConstMap[CmpLHS] = RHSConst;<br>
+ }<br>
+ }<br>
+ return Changed;<br>
+}<br>
static void
patchReplacementInstruction(Instruction *I, Value
*Repl) {<br>
// Patch the replacement so that it is not more
restrictive than the value<br>
@@ -2032,6 +2070,21 @@ static bool
isOnlyReachableViaThisEdge(c<br>
return Pred != nullptr;<br>
}<br>
+// Tries to replace instruction with const, using
information from<br>
+// ReplaceWithConstMap.<br>
+bool GVN::replaceOperandsWithConsts(Instruction
*Instr) const {<br>
+ bool Changed = false;<br>
+ for (unsigned OpNum = 0; OpNum <
Instr->getNumOperands(); ++OpNum) {<br>
+ Value *Operand = Instr->getOperand(OpNum);<br>
+ auto it = ReplaceWithConstMap.find(Operand);<br>
+ if (it != ReplaceWithConstMap.end()) {<br>
+ Instr->setOperand(OpNum, it->second);<br>
+ Changed = true;<br>
+ }<br>
+ }<br>
+ return Changed;<br>
+}<br>
+<br>
/// The given values are known to be equal in every
block<br>
/// dominated by 'Root'. Exploit this, for example
by replacing 'LHS' with<br>
/// 'RHS' everywhere in the scope. Returns whether
a change was made.<br>
@@ -2048,11 +2101,13 @@ bool
GVN::propagateEquality(Value *LHS,<br>
std::pair<Value*, Value*> Item =
Worklist.pop_back_val();<br>
LHS = Item.first; RHS = Item.second;<br>
- if (LHS == RHS) continue;<br>
+ if (LHS == RHS)<br>
+ continue;<br>
assert(LHS->getType() == RHS->getType()
&& "Equality but unequal types!");<br>
// Don't try to propagate equalities between
constants.<br>
- if (isa<Constant>(LHS) &&
isa<Constant>(RHS)) continue;<br>
+ if (isa<Constant>(LHS) &&
isa<Constant>(RHS))<br>
+ continue;<br>
// Prefer a constant on the right-hand side,
or an Argument if no constants.<br>
if (isa<Constant>(LHS) ||
(isa<Argument>(LHS) &&
!isa<Constant>(RHS)))<br>
@@ -2203,6 +2258,10 @@ bool
GVN::processInstruction(Instruction<br>
return true;<br>
}<br>
+ if (IntrinsicInst *IntrinsicI =
dyn_cast<IntrinsicInst>(I))<br>
+ if (IntrinsicI->getIntrinsicID() ==
Intrinsic::assume)<br>
+ return processAssumeIntrinsic(IntrinsicI);<br>
+<br>
if (LoadInst *LI = dyn_cast<LoadInst>(I)) {<br>
if (processLoad(LI))<br>
return true;<br>
@@ -2267,7 +2326,8 @@ bool
GVN::processInstruction(Instruction<br>
// Instructions with void type don't return a
value, so there's<br>
// no point in trying to find redundancies in
them.<br>
- if (I->getType()->isVoidTy()) return false;<br>
+ if (I->getType()->isVoidTy())<br>
+ return false;<br>
uint32_t NextNum =
VN.getNextUnusedValueNumber();<br>
unsigned Num = VN.lookup_or_add(I);<br>
@@ -2374,10 +2434,15 @@ bool
GVN::processBlock(BasicBlock *BB) {<br>
if (DeadBlocks.count(BB))<br>
return false;<br>
+ // Clearing map before every BB because it can be
used only for single BB.<br>
+ ReplaceWithConstMap.clear();<br>
bool ChangedFunction = false;<br>
for (BasicBlock::iterator BI = BB->begin(),
BE = BB->end();<br>
BI != BE;) {<br>
+ if (!ReplaceWithConstMap.empty())<br>
+ ChangedFunction |=
replaceOperandsWithConsts(BI);<br>
+<br>
ChangedFunction |= processInstruction(BI);<br>
if (InstrsToErase.empty()) {<br>
++BI;<br>
<br>
Added: llvm/trunk/test/Transforms/GVN/assume-equal.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/assume-equal.ll?rev=246243&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/assume-equal.ll?rev=246243&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/GVN/assume-equal.ll
(added)<br>
+++ llvm/trunk/test/Transforms/GVN/assume-equal.ll Thu
Aug 27 20:01:57 2015<br>
@@ -0,0 +1,122 @@<br>
+; RUN: opt < %s -gvn -S | FileCheck %s<br>
+<br>
+%struct.A = type { i32 (...)** }<br>
+@_ZTV1A = available_externally unnamed_addr constant
[4 x i8*] [i8* null, i8* bitcast (i8** @_ZTI1A to
i8*), i8* bitcast (i32 (%struct.A*)* @_ZN1A3fooEv to
i8*), i8* bitcast (i32 (%struct.A*)* @_ZN1A3barEv to
i8*)], align 8<br>
+@_ZTI1A = external constant i8*<br>
+<br>
+; Checks if indirect calls can be replaced with
direct<br>
+; assuming that %vtable == @_ZTV1A (with alignment).<br>
+; Checking const propagation across other BBs<br>
+; CHECK-LABEL: define void @_Z1gb(<br>
+<br>
+define void @_Z1gb(i1 zeroext %p) {<br>
+entry:<br>
+ %call = tail call noalias i8* @_Znwm(i64 8) #4<br>
+ %0 = bitcast i8* %call to %struct.A*<br>
+ tail call void @_ZN1AC1Ev(%struct.A* %0) #1<br>
+ %1 = bitcast i8* %call to i8***<br>
+ %vtable = load i8**, i8*** %1, align 8<br>
+ %cmp.vtables = icmp eq i8** %vtable, getelementptr
inbounds ([4 x i8*], [4 x i8*]* @_ZTV1A, i64 0, i64 2)<br>
+ tail call void @llvm.assume(i1 %cmp.vtables)<br>
+ br i1 %p, label %if.then, label %if.else<br>
+<br>
+if.then: ;
preds = %entry<br>
+ %vtable1.cast = bitcast i8** %vtable to i32
(%struct.A*)**<br>
+ %2 = load i32 (%struct.A*)*, i32 (%struct.A*)**
%vtable1.cast, align 8<br>
+<br>
+ ; CHECK: call i32 @_ZN1A3fooEv(<br>
+ %call2 = tail call i32 %2(%struct.A* %0) #1<br>
+<br>
+ br label %if.end<br>
+<br>
+if.else: ;
preds = %entry<br>
+ %vfn47 = getelementptr inbounds i8*, i8** %vtable,
i64 1<br>
+ %vfn4 = bitcast i8** %vfn47 to i32 (%struct.A*)**<br>
+<br>
+ ; CHECK: call i32 @_ZN1A3barEv(<br>
+ %3 = load i32 (%struct.A*)*, i32 (%struct.A*)**
%vfn4, align 8<br>
+<br>
+ %call5 = tail call i32 %3(%struct.A* %0) #1<br>
+ br label %if.end<br>
+<br>
+if.end: ;
preds = %if.else, %if.then<br>
+ ret void<br>
+}<br>
+<br>
+; Checking const propagation in the same BB<br>
+; CHECK-LABEL: define i32 @main()<br>
+<br>
+define i32 @main() {<br>
+entry:<br>
+ %call = tail call noalias i8* @_Znwm(i64 8)<br>
+ %0 = bitcast i8* %call to %struct.A*<br>
+ tail call void @_ZN1AC1Ev(%struct.A* %0)<br>
+ %1 = bitcast i8* %call to i8***<br>
+ %vtable = load i8**, i8*** %1, align 8<br>
+ %cmp.vtables = icmp eq i8** %vtable, getelementptr
inbounds ([4 x i8*], [4 x i8*]* @_ZTV1A, i64 0, i64 2)<br>
+ tail call void @llvm.assume(i1 %cmp.vtables)<br>
+ %vtable1.cast = bitcast i8** %vtable to i32
(%struct.A*)**<br>
+<br>
+ ; CHECK: call i32 @_ZN1A3fooEv(<br>
+ %2 = load i32 (%struct.A*)*, i32 (%struct.A*)**
%vtable1.cast, align 8<br>
+<br>
+ %call2 = tail call i32 %2(%struct.A* %0)<br>
+ ret i32 0<br>
+}<br>
+<br>
+; This tests checks const propatation with fcmp
instruction.<br>
+; CHECK-LABEL: define float @_Z1gf(float %p)<br>
+<br>
+define float @_Z1gf(float %p) {<br>
+entry:<br>
+ %p.addr = alloca float, align 4<br>
+ %f = alloca float, align 4<br>
+ store float %p, float* %p.addr, align 4<br>
+<br>
+ store float 3.000000e+00, float* %f, align 4<br>
+ %0 = load float, float* %p.addr, align 4<br>
+ %1 = load float, float* %f, align 4<br>
+ %cmp = fcmp oeq float %1, %0 ; note const on lhs<br>
+ call void @llvm.assume(i1 %cmp)<br>
+<br>
+ ; CHECK: ret float 3.000000e+00<br>
+ ret float %0<br>
+}<br>
+<br>
+; CHECK-LABEL: define float @_Z1hf(float %p)<br>
+<br>
+define float @_Z1hf(float %p) {<br>
+entry:<br>
+ %p.addr = alloca float, align 4<br>
+ store float %p, float* %p.addr, align 4<br>
+<br>
+ %0 = load float, float* %p.addr, align 4<br>
+ %cmp = fcmp nnan ueq float %0, 3.000000e+00<br>
+ call void @llvm.assume(i1 %cmp)<br>
+<br>
+ ; CHECK: ret float 3.000000e+00<br>
+ ret float %0<br>
+}<br>
+<br>
+; CHECK-LABEL: define float @_Z1if(float %p)<br>
+<br>
+<br>
+define float @_Z1if(float %p) {<br>
+entry:<br>
+ %p.addr = alloca float, align 4<br>
+ store float %p, float* %p.addr, align 4<br>
+<br>
+ %0 = load float, float* %p.addr, align 4<br>
+ %cmp = fcmp ueq float %0, 3.000000e+00 ; no nnan
flag - can't propagate<br>
+ call void @llvm.assume(i1 %cmp)<br>
+<br>
+ ; CHECK-NOT: ret float 3.000000e+00<br>
+ ret float %0<br>
+}<br>
+<br>
+declare noalias i8* @_Znwm(i64)<br>
+declare void @_ZN1AC1Ev(%struct.A*)<br>
+declare void @llvm.assume(i1)<br>
+declare i32 @_ZN1A3fooEv(%struct.A*)<br>
+declare i32 @_ZN1A3barEv(%struct.A*)<br>
+<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</div></div></div>
</blockquote></div><br></div>