<div dir="ltr"><div>Thanks, Philip.</div><div><br></div>Yes, I noticed your comments to the thread. I will follow-up there.<div><br></div><div>Farhana</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 19, 2018 at 11:39 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">FYI, I replied to the review thread with a few ideas for improvement on this patch.<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
On 07/19/2018 09:50 AM, Farhana Aleen via llvm-commits wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: faaleen<br>
Date: Thu Jul 19 09:50:27 2018<br>
New Revision: 337471<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=337471&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=337471&view=rev</a><br>
Log:<br>
[LoadStoreVectorizer] Use getMinusScev() to compute the distance between two pointers.<br>
<br>
Summary: Currently, isConsecutiveAccess() detects two pointers(PtrA and PtrB) as consecutive by<br>
          comparing PtrB with BaseDelta+PtrA. This works when both pointers are factorized or<br>
          both of them are not factorized. But isConsecutiveAccess() fails if one of the<br>
          pointers is factorized but the other one is not.<br>
<br>
          Here is an example:<br>
          PtrA = 4 * (A + B)<br>
          PtrB = 4 + 4A + 4B<br>
<br>
          This patch uses getMinusSCEV() to compute the distance between two pointers.<br>
          getMinusSCEV() allows combining the expressions and computing the simplified distance.<br>
<br>
Author: FarhanaAleen<br>
<br>
Reviewed By: rampitec<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D49516" rel="noreferrer" target="_blank">https://reviews.llvm.org/D4951<wbr>6</a><br>
<br>
Added:<br>
     llvm/trunk/test/Transforms/Lo<wbr>adStoreVectorizer/AMDGPU/compl<wbr>ex-index.ll<br>
Modified:<br>
     llvm/trunk/lib/Transforms/Vec<wbr>torize/LoadStoreVectorizer.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/Vect<wbr>orize/LoadStoreVectorizer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp?rev=337471&r1=337470&r2=337471&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Transform<wbr>s/Vectorize/LoadStoreVectorize<wbr>r.cpp?rev=337471&r1=337470&r2=<wbr>337471&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Transforms/Vect<wbr>orize/LoadStoreVectorizer.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Vect<wbr>orize/LoadStoreVectorizer.cpp Thu Jul 19 09:50:27 2018<br>
@@ -340,6 +340,14 @@ bool Vectorizer::isConsecutiveAcces<wbr>s(Val<br>
    if (X == PtrSCEVB)<br>
      return true;<br>
  +  // The above check will not catch the cases where one of the pointers is<br>
+  // factorized but the other one is not, such as (C + (S * (A + B))) vs<br>
+  // (AS + BS). Get the minus scev. That will allow re-combining the expresions<br>
+  // and getting the simplified difference.<br>
+  const SCEV *Dist = SE.getMinusSCEV(PtrSCEVB, PtrSCEVA);<br>
+  if (C == Dist)<br>
+    return true;<br>
+<br>
    // Sometimes even this doesn't work, because SCEV can't always see through<br>
    // patterns that look like (gep (ext (add (shl X, C1), C2))). Try checking<br>
    // things the hard way.<br>
<br>
Added: llvm/trunk/test/Transforms/Loa<wbr>dStoreVectorizer/AMDGPU/comple<wbr>x-index.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoadStoreVectorizer/AMDGPU/complex-index.ll?rev=337471&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/Transfor<wbr>ms/LoadStoreVectorizer/AMDGPU/<wbr>complex-index.ll?rev=337471&vi<wbr>ew=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Transforms/Loa<wbr>dStoreVectorizer/AMDGPU/comple<wbr>x-index.ll (added)<br>
+++ llvm/trunk/test/Transforms/Loa<wbr>dStoreVectorizer/AMDGPU/comple<wbr>x-index.ll Thu Jul 19 09:50:27 2018<br>
@@ -0,0 +1,49 @@<br>
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -basicaa -load-store-vectorizer -S -o - %s | FileCheck %s<br>
+<br>
+declare i64 @_Z12get_local_idj(i32)<br>
+<br>
+declare i64 @_Z12get_group_idj(i32)<br>
+<br>
+declare double @llvm.fmuladd.f64(double, double, double)<br>
+<br>
+; CHECK-LABEL: @factorizedVsNonfactorizedAcce<wbr>ss(<br>
+; CHECK: load <2 x float><br>
+; CHECK: store <2 x float><br>
+define amdgpu_kernel void @factorizedVsNonfactorizedAcce<wbr>ss(float addrspace(1)* nocapture %c) {<br>
+entry:<br>
+  %call = tail call i64 @_Z12get_local_idj(i32 0)<br>
+  %call1 = tail call i64 @_Z12get_group_idj(i32 0)<br>
+  %div = lshr i64 %call, 4<br>
+  %div2 = lshr i64 %call1, 3<br>
+  %mul = shl i64 %div2, 7<br>
+  %rem = shl i64 %call, 3<br>
+  %mul3 = and i64 %rem, 120<br>
+  %add = or i64 %mul, %mul3<br>
+  %rem4 = shl i64 %call1, 7<br>
+  %mul5 = and i64 %rem4, 896<br>
+  %mul6 = shl nuw nsw i64 %div, 3<br>
+  %add7 = add nuw i64 %mul5, %mul6<br>
+  %mul9 = shl i64 %add7, 10<br>
+  %add10 = add i64 %mul9, %add<br>
+  %arrayidx = getelementptr inbounds float, float addrspace(1)* %c, i64 %add10<br>
+  %load1 = load float, float addrspace(1)* %arrayidx, align 4<br>
+  %conv = fpext float %load1 to double<br>
+  %mul11 = fmul double %conv, 0x3FEAB481D8F35506<br>
+  %conv12 = fptrunc double %mul11 to float<br>
+  %conv18 = fpext float %conv12 to double<br>
+  %storeval1 = tail call double @llvm.fmuladd.f64(double 0x3FF4FFAFBBEC946A, double 0.000000e+00, double %conv18)<br>
+  %cstoreval1 = fptrunc double %storeval1 to float<br>
+  store float %cstoreval1, float addrspace(1)* %arrayidx, align 4<br>
+<br>
+  %add23 = or i64 %add10, 1<br>
+  %arrayidx24 = getelementptr inbounds float, float addrspace(1)* %c, i64 %add23<br>
+  %load2 = load float, float addrspace(1)* %arrayidx24, align 4<br>
+  %conv25 = fpext float %load2 to double<br>
+  %mul26 = fmul double %conv25, 0x3FEAB481D8F35506<br>
+  %conv27 = fptrunc double %mul26 to float<br>
+  %conv34 = fpext float %conv27 to double<br>
+  %storeval2 = tail call double @llvm.fmuladd.f64(double 0x3FF4FFAFBBEC946A, double 0.000000e+00, double %conv34)<br>
+  %cstoreval2 = fptrunc double %storeval2 to float<br>
+  store float %cstoreval2, float addrspace(1)* %arrayidx24, align 4<br>
+  ret void<br>
+}<br>
\ No newline at end of file<br>
<br>
<br>
______________________________<wbr>_________________<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/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote>
<br>
</div></div></blockquote></div><br></div>