<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<br>
<div class="moz-cite-prefix">On 07/08/2015 10:29 AM, Jingyue Wu
wrote:<br>
</div>
<blockquote
cite="mid:CAMROOrGwmP4vt2i+CVGWV8tEvmywdZQ9xwr_H-mWWKBESP1TWA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>Hi Philip, </div>
<div><br>
</div>
<div>The reason that I added this rule in NaryReassociate
instead of InstCombine is that InstCombine happens too early
for this rule to be useful for our cases. In our cases, this</div>
<div><br>
</div>
<div>a[i]</div>
<div>a[i + j]</div>
<div><br>
</div>
<div>pattern occurs after certain iterations of NaryReassociate,
so I have to put it over there. </div>
<div><br>
</div>
<div>I like the idea of putting the cover function in
ValueTracking, and I noticed there are some similar hosts for
unsigned operations already such as
computeOverflowForUnsignedAdd (<a moz-do-not-send="true" href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_docs_doxygen_html_ValueTracking-5F8h-5Fsource.html-23l00286&d=AwMDaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=JcpN40eXlNnE1U822W3iB6zQJn6LKR6zFA1MRWp1xX8&s=PFUutPS6Q4vD3s2SMqh33SE6E_mrBDLhLHPEHMoKyEs&e=">http://llvm.org/docs/doxygen/html/ValueTracking_8h_source.html#l00286</a>).
I'd probably need to create a new interface
computeOverflowForSignedAdd, and add the rule</div>
<div><br>
</div>
<div>i >= 0 && i + j >= 0 => i +nsw j</div>
<div><br>
</div>
<div>Then, InstCombine can strengthen nsw using this interface.
I haven't seen a particular use case that benefits from doing
this in InstCombine, but I believe it's good in general. </div>
<div><br>
</div>
<div>Sounds good to you? <br>
</div>
</div>
</blockquote>
Generally plan seems reasonable. Sorry for delayed response.<br>
<blockquote
cite="mid:CAMROOrGwmP4vt2i+CVGWV8tEvmywdZQ9xwr_H-mWWKBESP1TWA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div><br>
</div>
<div>Jingyue</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Jul 8, 2015 at 9:56 AM, Philip
Reames <span dir="ltr"><<a moz-do-not-send="true" 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">Sorry for
the delayed comment, just got back to this in my email.<br>
<br>
Looking at the patch, I was wondering why the rule to add
nsw wasn't part of instcombine? Is there a reasoning why we
can assume nsw for purposes of reassociation, but not add
the generic attribute? Doing this in instcombine would seem
to be generally useful.<br>
<br>
Comment inline as well.
<div>
<div class="h5"><br>
<br>
On 06/30/2015 08:38 PM, Jingyue Wu wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: jingyue<br>
Date: Tue Jun 30 22:38:49 2015<br>
New Revision: 241139<br>
<br>
URL: <a moz-do-not-send="true" href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D241139-26view-3Drev&d=AwMDaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=JcpN40eXlNnE1U822W3iB6zQJn6LKR6zFA1MRWp1xX8&s=vFpPw1YG9cxsrMUKufZ2aTW01r4qWcFxAoZDTz3_xws&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=241139&view=rev</a><br>
Log:<br>
[NaryReassociate] enhances nsw by leveraging
@llvm.assume<br>
<br>
Summary:<br>
nsw are flaky and can often be removed by
optimizations. This patch enhances<br>
nsw by leveraging @llvm.assume in the IR.
Specifically, NaryReassociate now<br>
understands that<br>
<br>
assume(a + b >= 0) && assume(a >=
0) ==> a +nsw b<br>
<br>
As a result, it can split more sext(a + b) into
sext(a) + sext(b) for CSE.<br>
<br>
Test Plan: nary-gep.ll<br>
<br>
Reviewers: broune, meheff<br>
<br>
Subscribers: jholewinski, llvm-commits<br>
<br>
Differential Revision: <a moz-do-not-send="true" href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10822&d=AwMDaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=JcpN40eXlNnE1U822W3iB6zQJn6LKR6zFA1MRWp1xX8&s=8gWafJx6uITFMkCbajQwOEWej7T5UwyhHRbL9AEVshg&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10822</a><br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp<br>
llvm/trunk/test/Transforms/NaryReassociate/NVPTX/nary-gep.ll<br>
<br>
Modified:
llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp<br>
URL: <a moz-do-not-send="true" href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_Scalar_NaryReassociate.cpp-3Frev-3D241139-26r1-3D241138-26r2-3D241139-26view-3Ddiff&d=AwMDaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=JcpN40eXlNnE1U822W3iB6zQJn6LKR6zFA1MRWp1xX8&s=G33LitNzrR6ASOk_LV9i77vePMrTaUf5fblKQzsPDzw&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp?rev=241139&r1=241138&r2=241139&view=diff</a><br>
==============================================================================<br>
---
llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp
(original)<br>
+++
llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp
Tue Jun 30 22:38:49 2015<br>
@@ -74,21 +74,18 @@<br>
// 1) We only considers n-ary adds for now. This
should be extended and<br>
// generalized.<br>
//<br>
-// 2) Besides arithmetic operations, similar
reassociation can be applied to<br>
-// GEPs. For example, if<br>
-// X = &arr[a]<br>
-// dominates<br>
-// Y = &arr[a + b]<br>
-// we may rewrite Y into X + b.<br>
-//<br>
//===----------------------------------------------------------------------===//<br>
+#include "llvm/Analysis/AssumptionCache.h"<br>
#include "llvm/Analysis/ScalarEvolution.h"<br>
#include "llvm/Analysis/TargetLibraryInfo.h"<br>
#include "llvm/Analysis/TargetTransformInfo.h"<br>
+#include "llvm/Analysis/ValueTracking.h"<br>
#include "llvm/IR/Dominators.h"<br>
#include "llvm/IR/Module.h"<br>
#include "llvm/IR/PatternMatch.h"<br>
+#include "llvm/Support/Debug.h"<br>
+#include "llvm/Support/raw_ostream.h"<br>
#include "llvm/Transforms/Scalar.h"<br>
#include "llvm/Transforms/Utils/Local.h"<br>
using namespace llvm;<br>
@@ -115,6 +112,7 @@ public:<br>
AU.addPreserved<DominatorTreeWrapperPass>();<br>
AU.addPreserved<ScalarEvolution>();<br>
AU.addPreserved<TargetLibraryInfoWrapperPass>();<br>
+ AU.addRequired<AssumptionCacheTracker>();<br>
AU.addRequired<DominatorTreeWrapperPass>();<br>
AU.addRequired<ScalarEvolution>();<br>
AU.addRequired<TargetLibraryInfoWrapperPass>();<br>
@@ -163,12 +161,18 @@ private:<br>
// GEP's pointer size, i.e., whether Index needs
to be sign-extended in order<br>
// to be an index of GEP.<br>
bool requiresSignExtension(Value *Index,
GetElementPtrInst *GEP);<br>
+ // Returns whether V is known to be non-negative at
context \c Ctxt.<br>
+ bool isKnownNonNegative(Value *V, Instruction
*Ctxt);<br>
+ // Returns whether AO may sign overflow at context
\c Ctxt. It computes a<br>
+ // conservative result -- it answers true when not
sure.<br>
+ bool maySignOverflow(AddOperator *AO, Instruction
*Ctxt);<br>
+ AssumptionCache *AC;<br>
+ const DataLayout *DL;<br>
DominatorTree *DT;<br>
ScalarEvolution *SE;<br>
TargetLibraryInfo *TLI;<br>
TargetTransformInfo *TTI;<br>
- const DataLayout *DL;<br>
// A lookup table quickly telling which
instructions compute the given SCEV.<br>
// Note that there can be multiple instructions at
different locations<br>
// computing to the same SCEV, so we map a SCEV to
an instruction list. For<br>
@@ -185,6 +189,7 @@ private:<br>
char NaryReassociate::ID = 0;<br>
INITIALIZE_PASS_BEGIN(NaryReassociate,
"nary-reassociate", "Nary reassociation",<br>
false, false)<br>
+INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)<br>
INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)<br>
INITIALIZE_PASS_DEPENDENCY(ScalarEvolution)<br>
INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)<br>
@@ -200,6 +205,7 @@ bool
NaryReassociate::runOnFunction(Func<br>
if (skipOptnoneFunction(F))<br>
return false;<br>
+ AC =
&getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);<br>
DT =
&getAnalysis<DominatorTreeWrapperPass>().getDomTree();<br>
SE = &getAnalysis<ScalarEvolution>();<br>
TLI =
&getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();<br>
@@ -346,18 +352,44 @@ bool
NaryReassociate::requiresSignExtens<br>
return
cast<IntegerType>(Index->getType())->getBitWidth()
< PointerSizeInBits;<br>
}<br>
+bool NaryReassociate::isKnownNonNegative(Value *V,
Instruction *Ctxt) {<br>
+ bool NonNegative, Negative;<br>
+ // TODO: ComputeSignBits is expensive. Consider
caching the results.<br>
+ ComputeSignBit(V, NonNegative, Negative, *DL, 0,
AC, Ctxt, DT);<br>
+ return NonNegative;<br>
+}<br>
</blockquote>
</div>
</div>
This really seems like a cover function which should live in
ValueTracking.
<div class="HOEnZb">
<div class="h5"><br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+bool NaryReassociate::maySignOverflow(AddOperator
*AO, Instruction *Ctxt) {<br>
+ if (AO->hasNoSignedWrap())<br>
+ return false;<br>
+<br>
+ Value *LHS = AO->getOperand(0), *RHS =
AO->getOperand(1);<br>
+ // If LHS or RHS has the same sign as the sum, AO
doesn't sign overflow.<br>
+ // TODO: handle the negative case as well.<br>
+ if (isKnownNonNegative(AO, Ctxt) &&<br>
+ (isKnownNonNegative(LHS, Ctxt) ||
isKnownNonNegative(RHS, Ctxt)))<br>
+ return false;<br>
+<br>
+ return true;<br>
+}<br>
+<br>
GetElementPtrInst *<br>
NaryReassociate::tryReassociateGEPAtIndex(GetElementPtrInst
*GEP, unsigned I,<br>
Type
*IndexedType) {<br>
Value *IndexToSplit = GEP->getOperand(I + 1);<br>
- if (SExtInst *SExt =
dyn_cast<SExtInst>(IndexToSplit))<br>
+ if (SExtInst *SExt =
dyn_cast<SExtInst>(IndexToSplit)) {<br>
IndexToSplit = SExt->getOperand(0);<br>
+ } else if (ZExtInst *ZExt =
dyn_cast<ZExtInst>(IndexToSplit)) {<br>
+ // zext can be treated as sext if the source is
non-negative.<br>
+ if (isKnownNonNegative(ZExt->getOperand(0),
GEP))<br>
+ IndexToSplit = ZExt->getOperand(0);<br>
+ }<br>
if (AddOperator *AO =
dyn_cast<AddOperator>(IndexToSplit)) {<br>
// If the I-th index needs sext and the
underlying add is not equipped with<br>
// nsw, we cannot split the add because<br>
// sext(LHS + RHS) != sext(LHS) + sext(RHS).<br>
- if (requiresSignExtension(IndexToSplit, GEP)
&& !AO->hasNoSignedWrap())<br>
+ if (requiresSignExtension(IndexToSplit, GEP)
&& maySignOverflow(AO, GEP))<br>
return nullptr;<br>
Value *LHS = AO->getOperand(0), *RHS =
AO->getOperand(1);<br>
// IndexToSplit = LHS + RHS.<br>
@@ -373,10 +405,9 @@
NaryReassociate::tryReassociateGEPAtInde<br>
return nullptr;<br>
}<br>
-GetElementPtrInst *<br>
-NaryReassociate::tryReassociateGEPAtIndex(GetElementPtrInst
*GEP, unsigned I,<br>
- Value *LHS,
Value *RHS,<br>
- Type
*IndexedType) {<br>
+GetElementPtrInst
*NaryReassociate::tryReassociateGEPAtIndex(<br>
+ GetElementPtrInst *GEP, unsigned I, Value *LHS,
Value *RHS,<br>
+ Type *IndexedType) {<br>
// Look for GEP's closest dominator that has the
same SCEV as GEP except that<br>
// the I-th index is replaced with LHS.<br>
SmallVector<const SCEV *, 4> IndexExprs;<br>
@@ -384,6 +415,16 @@
NaryReassociate::tryReassociateGEPAtInde<br>
IndexExprs.push_back(SE->getSCEV(*Index));<br>
// Replace the I-th index with LHS.<br>
IndexExprs[I] = SE->getSCEV(LHS);<br>
+ if (isKnownNonNegative(LHS, GEP) &&<br>
+ DL->getTypeSizeInBits(LHS->getType())
<<br>
+
DL->getTypeSizeInBits(GEP->getOperand(I)->getType()))
{<br>
+ // Zero-extend LHS if it is non-negative.
InstCombine canonicalizes sext to<br>
+ // zext if the source operand is proved
non-negative. We should do that<br>
+ // consistently so that CandidateExpr more likely
appears before. See<br>
+ // @reassociate_gep_assume for an example of this
canonicalization.<br>
+ IndexExprs[I] =<br>
+ SE->getZeroExtendExpr(IndexExprs[I],
GEP->getOperand(I)->getType());<br>
+ }<br>
const SCEV *CandidateExpr = SE->getGEPExpr(<br>
GEP->getSourceElementType(),
SE->getSCEV(GEP->getPointerOperand()),<br>
IndexExprs, GEP->isInBounds());<br>
<br>
Modified:
llvm/trunk/test/Transforms/NaryReassociate/NVPTX/nary-gep.ll<br>
URL: <a moz-do-not-send="true" href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_Transforms_NaryReassociate_NVPTX_nary-2Dgep.ll-3Frev-3D241139-26r1-3D241138-26r2-3D241139-26view-3Ddiff&d=AwMDaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=JcpN40eXlNnE1U822W3iB6zQJn6LKR6zFA1MRWp1xX8&s=AVvP2EZNS0CksVjin6c-EMV5Fq8XT0b_9Ba28hHXKLA&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NaryReassociate/NVPTX/nary-gep.ll?rev=241139&r1=241138&r2=241139&view=diff</a><br>
==============================================================================<br>
---
llvm/trunk/test/Transforms/NaryReassociate/NVPTX/nary-gep.ll
(original)<br>
+++
llvm/trunk/test/Transforms/NaryReassociate/NVPTX/nary-gep.ll
Tue Jun 30 22:38:49 2015<br>
@@ -61,6 +61,40 @@ define void
@reassociate_gep_nsw(float*<br>
ret void<br>
}<br>
+; assume(j >= 0);<br>
+; foo(&a[zext(j)]);<br>
+; assume(i + j >= 0);<br>
+; foo(&a[zext(i + j)]);<br>
+; =><br>
+; t1 = &a[zext(j)];<br>
+; foo(t1);<br>
+; t2 = t1 + sext(i);<br>
+; foo(t2);<br>
+define void @reassociate_gep_assume(float* %a, i32
%i, i32 %j) {<br>
+; CHECK-LABEL: @reassociate_gep_assume(<br>
+ ; assume(j >= 0)<br>
+ %cmp = icmp sgt i32 %j, -1<br>
+ call void @llvm.assume(i1 %cmp)<br>
+ %1 = add i32 %i, %j<br>
+ %cmp2 = icmp sgt i32 %1, -1<br>
+ call void @llvm.assume(i1 %cmp2)<br>
+<br>
+ %idxprom.j = zext i32 %j to i64<br>
+ %2 = getelementptr float, float* %a, i64 %idxprom.j<br>
+; CHECK: [[t1:[^ ]+]] = getelementptr float, float*
%a, i64 %idxprom.j<br>
+ call void @foo(float* %2)<br>
+; CHECK: call void @foo(float* [[t1]])<br>
+<br>
+ %idxprom.1 = zext i32 %1 to i64<br>
+ %3 = getelementptr float, float* %a, i64 %idxprom.1<br>
+; CHECK: [[sexti:[^ ]+]] = sext i32 %i to i64<br>
+; CHECK: [[t2:[^ ]+]] = getelementptr float, float*
[[t1]], i64 [[sexti]]<br>
+ call void @foo(float* %3)<br>
+; CHECK: call void @foo(float* [[t2]])<br>
+<br>
+ ret void<br>
+}<br>
+<br>
; Do not split the second GEP because sext(i + j) !=
sext(i) + sext(j).<br>
define void @reassociate_gep_no_nsw(float* %a, i32
%i, i32 %j) {<br>
; CHECK-LABEL: @reassociate_gep_no_nsw(<br>
@@ -88,3 +122,5 @@ define void
@reassociate_gep_128(float*<br>
; CHECK: call void @foo(float* [[t2]])<br>
ret void<br>
}<br>
+<br>
+declare void @llvm.assume(i1)<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a moz-do-not-send="true" href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a moz-do-not-send="true" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</body>
</html>