[PATCH] D104403: [SCEV] Avoid pointer subtraction of non-integral pointers [WIP]

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: lebedev.ri, nikic, efriedma, mkazantsev.
Herald added subscribers: javed.absar, bollu, hiraditya, mcrosier.
reames requested review of this revision.
Herald added a project: LLVM.

This is a WIP, mostly posted for discussion and performance evaluation.

Over in D104322 <https://reviews.llvm.org/D104322>, and D103660 <https://reviews.llvm.org/D103660>, and a couple other related reviews, there's a desire to avoid pointer math in SCEV.  For normal (integral) pointers, we can do this by pushing ptrtoint down expression trees, but we don't have that option for non-integral ones.  This patch tries to minimally cripple SCEVs ability to reason about non-integrals, while still allowing forward progress on that idea.

The main impact of this is, from what I can tell, hurting the vectorizers ability to version loops and SCEV-AA.  Note that handling ptr - int is important, without that we'd also loose LSR and that's a pretty much fatal flaw.

I'd like to see a performance validation of this patch.  @mkazantsev, @skatkov - Can either of you help with that?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104403

Files:
  llvm/lib/Analysis/LoopAccessAnalysis.cpp
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/test/Analysis/LoopAccessAnalysis/wrapping-pointer-ni.ll


Index: llvm/test/Analysis/LoopAccessAnalysis/wrapping-pointer-ni.ll
===================================================================
--- llvm/test/Analysis/LoopAccessAnalysis/wrapping-pointer-ni.ll
+++ llvm/test/Analysis/LoopAccessAnalysis/wrapping-pointer-ni.ll
@@ -12,20 +12,15 @@
 
 declare i64 @julia_steprange_last_4949()
 
+ at G = external global i32
+
 define void @"japi1_align!_9477"(%jl_value_t addrspace(10)** %arg) {
 ; LV-LAVEL: L26.lver.check
-; LV: [[OFMul:%[^ ]*]]  = call { i64, i1 } @llvm.umul.with.overflow.i64(i64 4, i64 [[Step:%[^ ]*]])
-; LV-NEXT: [[OFMulResult:%[^ ]*]] = extractvalue { i64, i1 } [[OFMul]], 0
-; LV-NEXT: [[OFMulOverflow:%[^ ]*]] = extractvalue { i64, i1 } [[OFMul]], 1
-; LV-NEXT: [[PosGEP:%[^ ]*]] = getelementptr i32, i32 addrspace(13)* [[Base:%[^ ]*]], i64 [[Step]]
-; LV-NEXT: [[NegGEP:%[^ ]*]] = getelementptr i32, i32 addrspace(13)* [[Base]], i64 [[NegStep:%[^ ]*]]
-; LV-NEXT: icmp ugt i32 addrspace(13)* [[NegGEP]], [[Base]]
-; LV-NEXT: icmp ult i32 addrspace(13)* [[PosGEP]], [[Base]]
 ; LV-NOT: inttoptr
 ; LV-NOT: ptrtoint
 top:
   %tmp = load %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %arg, align 8
-  %tmp1 = load i32, i32* inttoptr (i64 12 to i32*), align 4
+  %tmp1 = load i32, i32* @G, align 4
   %tmp2 = sub i32 0, %tmp1
   %tmp3 = call i64 @julia_steprange_last_4949()
   %tmp4 = addrspacecast %jl_value_t addrspace(10)* %tmp to %jl_value_t addrspace(11)*
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===================================================================
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -4085,6 +4085,16 @@
   if (LHS == RHS)
     return getZero(LHS->getType());
 
+  // Pointer subtraction when LHS and RHS are based on different objects is
+  // not well defined.  For integral pointers, we fall back to inttoptrs
+  // but we don't have that option for non-integral pointers. Note that
+  // subtracting an offset from a non-integral pointer is fine.
+  // TODO: If we had an actual pointer subtraction primitive which didn't
+  // require ptrtoint, we could get better results for non-integral pointers
+  // here.
+  if (getDataLayout().isNonIntegralPointerType(RHS->getType()))
+    return getCouldNotCompute();
+
   // We represent LHS - RHS as LHS + (-1)*RHS. This transformation
   // makes it so that we cannot make much use of NUW.
   auto AddFlags = SCEV::FlagAnyWrap;
Index: llvm/lib/Analysis/LoopAccessAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -1492,6 +1492,9 @@
   LLVM_DEBUG(dbgs() << "LAA: Distance for " << *InstMap[AIdx] << " to "
                     << *InstMap[BIdx] << ": " << *Dist << "\n");
 
+  if (isa<SCEVCouldNotCompute>(Dist))
+    return Dependence::Unknown;
+
   // Need accesses with constant stride. We don't want to vectorize
   // "A[B[i]] += ..." and similar code or pointer arithmetic that could wrap in
   // the address space.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104403.352482.patch
Type: text/x-patch
Size: 3042 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210616/80265a36/attachment.bin>