<div dir="ltr">(you had one in the first patch, but the commit doesnt' have it)<div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 10, 2017 at 1:16 PM, Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</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">Please add a testcase, too :)</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 10, 2017 at 1:16 PM, Phabricator via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This revision was automatically updated to reflect the committed changes.<br>
Closed by commit rL307581: Avoid doing conservative phi checks in aliasSameBasePointerGEPs() if no phis… (authored by faaleen).<br>
<br>
Changed prior to commit:<br>
<a href="https://reviews.llvm.org/D34478?vs=103471&id=105908#toc" rel="noreferrer" target="_blank">https://reviews.llvm.org/D3447<wbr>8?vs=103471&id=105908#toc</a><br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D34478" rel="noreferrer" target="_blank">https://reviews.llvm.org/D3447<wbr>8</a><br>
<br>
Files:<br>
llvm/trunk/include/llvm/Analys<wbr>is/BasicAliasAnalysis.h<br>
llvm/trunk/lib/Analysis/BasicA<wbr>liasAnalysis.cpp<br>
llvm/trunk/lib/Analysis/ValueT<wbr>racking.cpp<br>
<br>
<br>
Index: llvm/trunk/lib/Analysis/ValueT<wbr>racking.cpp<br>
==============================<wbr>==============================<wbr>=======<br>
--- llvm/trunk/lib/Analysis/ValueT<wbr>racking.cpp<br>
+++ llvm/trunk/lib/Analysis/ValueT<wbr>racking.cpp<br>
@@ -1873,7 +1873,7 @@<br>
if (Known.countMaxLeadingZeros() < BitWidth - ShiftVal)<br>
return true;<br>
// Are all the bits to be shifted out known zero?<br>
- if (Known.countMinTrailingZeros() >= ShiftVal)<br>
+ if (Known.isUnknown() || Known.countMinTrailingZeros() >= ShiftVal)<br>
return isKnownNonZero(X, Depth, Q);<br>
}<br>
}<br>
Index: llvm/trunk/lib/Analysis/BasicA<wbr>liasAnalysis.cpp<br>
==============================<wbr>==============================<wbr>=======<br>
--- llvm/trunk/lib/Analysis/BasicA<wbr>liasAnalysis.cpp<br>
+++ llvm/trunk/lib/Analysis/BasicA<wbr>liasAnalysis.cpp<br>
@@ -922,11 +922,11 @@<br>
<br>
/// Provide ad-hoc rules to disambiguate accesses through two GEP operators,<br>
/// both having the exact same pointer operand.<br>
-static AliasResult aliasSameBasePointerGEPs(const GEPOperator *GEP1,<br>
- uint64_t V1Size,<br>
- const GEPOperator *GEP2,<br>
- uint64_t V2Size,<br>
- const DataLayout &DL) {<br>
+AliasResult BasicAAResult::aliasSameBasePo<wbr>interGEPs(const GEPOperator *GEP1,<br>
+ uint64_t V1Size,<br>
+ const GEPOperator *GEP2,<br>
+ uint64_t V2Size,<br>
+ const DataLayout &DL) {<br>
<br>
assert(GEP1->getPointerOperan<wbr>d()->stripPointerCastsAndBarri<wbr>ers() ==<br>
GEP2->getPointerOperand()->str<wbr>ipPointerCastsAndBarriers() &&<br>
@@ -1006,19 +1006,23 @@<br>
// Because they cannot partially overlap and because fields in an array<br>
// cannot overlap, if we can prove the final indices are different between<br>
// GEP1 and GEP2, we can conclude GEP1 and GEP2 don't alias.<br>
-<br>
+<br>
// If the last indices are constants, we've already checked they don't<br>
// equal each other so we can exit early.<br>
if (C1 && C2)<br>
return NoAlias;<br>
{<br>
Value *GEP1LastIdx = GEP1->getOperand(GEP1->getNumO<wbr>perands() - 1);<br>
Value *GEP2LastIdx = GEP2->getOperand(GEP2->getNumO<wbr>perands() - 1);<br>
- if (isa<PHINode>(GEP1LastIdx) || isa<PHINode>(GEP2LastIdx)) {<br>
+ if ((isa<PHINode>(GEP1LastIdx) || isa<PHINode>(GEP2LastIdx)) &&<br>
+ !VisitedPhiBBs.empty()) {<br>
// If one of the indices is a PHI node, be safe and only use<br>
// computeKnownBits so we don't make any assumptions about the<br>
// relationships between the two indices. This is important if we're<br>
// asking about values from different loop iterations. See PR32314.<br>
+ // But, with empty visitedPhiBBs we can guarantee that the values are<br>
+ // from the same iteration. Therefore, we can avoid doing this<br>
+ // conservative check.<br>
// TODO: We may be able to change the check so we only do this when<br>
// we definitely looked through a PHINode.<br>
if (GEP1LastIdx != GEP2LastIdx &&<br>
Index: llvm/trunk/include/llvm/Analys<wbr>is/BasicAliasAnalysis.h<br>
==============================<wbr>==============================<wbr>=======<br>
--- llvm/trunk/include/llvm/Analys<wbr>is/BasicAliasAnalysis.h<br>
+++ llvm/trunk/include/llvm/Analys<wbr>is/BasicAliasAnalysis.h<br>
@@ -183,6 +183,12 @@<br>
uint64_t V2Size, const AAMDNodes &V2AAInfo,<br>
const Value *UnderlyingV1, const Value *UnderlyingV2);<br>
<br>
+ AliasResult aliasSameBasePointerGEPs(const GEPOperator *GEP1,<br>
+ uint64_t V1Size,<br>
+ const GEPOperator *GEP2,<br>
+ uint64_t V2Size,<br>
+ const DataLayout &DL);<br>
+<br>
AliasResult aliasPHI(const PHINode *PN, uint64_t PNSize,<br>
const AAMDNodes &PNAAInfo, const Value *V2,<br>
uint64_t V2Size, const AAMDNodes &V2AAInfo,<br>
<br>
<br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>