<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 28 August 2015 at 17:02, Piotr Padlewski <span dir="ltr"><<a href="mailto:prazek@google.com" target="_blank">prazek@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><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.<span class="HOEnZb"><font color="#888888"><div><br></div><div>Piotr</div></font></span></div><div class="HOEnZb"><div class="h5"><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></div></blockquote></div></div></div></div></blockquote><div><br></div><div>Exactly as written, the BB scan flag would always end up true in this case because we just hit a use in this BB. Assuming you fix this with "if use is in same BB and is not the assume instruction itself: set BB scan flag", then I think what you're buying is not filling in ReplaceWithConstMap as often, which is nice since we could sometimes avoid the operand scan for the rest of the instructions in the block.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">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><font color="#888888"><br>
<br>
Philip</font></span><div><div><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>
</div></div></blockquote></div><br></div></div>