[llvm] r282612 - Don't look through addrspacecast in GetPointerBaseWithConstantOffset

[Artur Pilipenko via llvm-commits]

Hi,

In my fix I don't look through addrspace casts which change pointer size. It’s just of one of a possible conservative fixes for the assert in GetPointerBaseWithConstantOffset. Since the semantics of different addrspaces is not defined, I guess we can look through them even if the pointer size is changed. If you have a scenario when this kind of optimization matters, you can proceed with your solution.

Note, that we have similar logic in Value::stripAndAccumulateInBoundsConstantOffsets and ValueTracking::isDereferenceableAndAlignedPointer. I have a patch for review which unifies their behavior. Can’t send a link right now because reviews.llvm.org<http://reviews.llvm.org> seems to be down. If you decide to look through addrspace cast even if it changes pointer size it will be nice to fix these two as well.

Artur

On 29 Sep 2016, at 00:12, Tom Stellard <tom at stellard.net<mailto:tom at stellard.net>> wrote:

On Wed, Sep 28, 2016 at 05:57:19PM -0000, Artur Pilipenko via llvm-commits wrote:
Author: apilipenko
Date: Wed Sep 28 12:57:16 2016
New Revision: 282612

URL: http://llvm.org/viewvc/llvm-project?rev=282612&view=rev
Log:
Don't look through addrspacecast in GetPointerBaseWithConstantOffset

Pointers in different addrspaces can have different sizes, so it's not valid to look through addrspace cast calculating base and offset for a value.


Hi,

Why is not valid to look through addrspacecasts?  I had proposed an alternative fix
for this problem here: https://reviews.llvm.org/D24772 which still allows this.

-Tom

This is similar to D13008.

Reviewed By: reames

Differential Revision: https://reviews.llvm.org/D24729

Added:
   llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll
   llvm/trunk/test/Transforms/GVN/addrspace-cast.ll
Modified:
   llvm/trunk/lib/Analysis/ValueTracking.cpp
   llvm/trunk/test/Transforms/GVN/PRE/rle.ll

Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=282612&r1=282611&r2=282612&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp Wed Sep 28 12:57:16 2016
@@ -2848,9 +2848,14 @@ Value *llvm::GetPointerBaseWithConstantO
      ByteOffset += GEPOffset;

      Ptr = GEP->getPointerOperand();
-    } else if (Operator::getOpcode(Ptr) == Instruction::BitCast ||
-               Operator::getOpcode(Ptr) == Instruction::AddrSpaceCast) {
+    } else if (Operator::getOpcode(Ptr) == Instruction::BitCast) {
      Ptr = cast<Operator>(Ptr)->getOperand(0);
+    } else if (AddrSpaceCastInst *ASCI = dyn_cast<AddrSpaceCastInst>(Ptr)) {
+      Value *SourcePtr = ASCI->getPointerOperand();
+      // Don't look through addrspace cast which changes pointer size
+      if (BitWidth != DL.getPointerTypeSizeInBits(SourcePtr->getType()))
+        break;
+      Ptr = SourcePtr;
    } else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(Ptr)) {
      if (GA->isInterposable())
        break;

Added: llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll?rev=282612&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll (added)
+++ llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll Wed Sep 28 12:57:16 2016
@@ -0,0 +1,14 @@
+; RUN: opt < %s -default-data-layout="e-p:32:32:32-p1:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-n8:16:32" -basicaa -gvn -S -die | FileCheck %s
+
+define i8 @coerce_offset0_addrspacecast(i32 %V, i32* %P) {
+  store i32 %V, i32* %P
+
+  %P2 = addrspacecast i32* %P to i8 addrspace(1)*
+  %P3 = getelementptr i8, i8 addrspace(1)* %P2, i32 2
+
+  %A = load i8, i8 addrspace(1)* %P3
+  ret i8 %A
+; CHECK-LABEL: @coerce_offset0_addrspacecast(
+; CHECK-NOT: load
+; CHECK: ret i8
+}

Modified: llvm/trunk/test/Transforms/GVN/PRE/rle.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/PRE/rle.ll?rev=282612&r1=282611&r2=282612&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/GVN/PRE/rle.ll (original)
+++ llvm/trunk/test/Transforms/GVN/PRE/rle.ll Wed Sep 28 12:57:16 2016
@@ -318,19 +318,6 @@ define i8 @coerce_offset0(i32 %V, i32* %
; CHECK: ret i8
}

-define i8 @coerce_offset0_addrspacecast(i32 %V, i32* %P) {
-  store i32 %V, i32* %P
-
-  %P2 = addrspacecast i32* %P to i8 addrspace(1)*
-  %P3 = getelementptr i8, i8 addrspace(1)* %P2, i32 2
-
-  %A = load i8, i8 addrspace(1)* %P3
-  ret i8 %A
-; CHECK-LABEL: @coerce_offset0_addrspacecast(
-; CHECK-NOT: load
-; CHECK: ret i8
-}
-
;; non-local i32/float -> i8 load forwarding.
define i8 @coerce_offset_nonlocal0(i32* %P, i1 %cond) {
  %P2 = bitcast i32* %P to float*

Added: llvm/trunk/test/Transforms/GVN/addrspace-cast.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/addrspace-cast.ll?rev=282612&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GVN/addrspace-cast.ll (added)
+++ llvm/trunk/test/Transforms/GVN/addrspace-cast.ll Wed Sep 28 12:57:16 2016
@@ -0,0 +1,21 @@
+; RUN: opt < %s -gvn -S | FileCheck %s
+target datalayout = "e-m:e-p:16:16-p1:32:16-i32:16-i64:16-n8:16"
+
+; In cases where two address spaces do not have the same size pointer, the
+; input for the addrspacecast should not be used as a substitute for itself
+; when manipulating the pointer.
+
+; Check that we don't hit the assert in this scenario
+define i8 @test(i32 %V, i32* %P) {
+; CHECK-LABEL: @test(
+; CHECK: load
+  %P1 = getelementptr inbounds i32, i32* %P, i16 16
+
+  store i32 %V, i32* %P1
+
+  %P2 = addrspacecast i32* %P1 to i8 addrspace(1)*
+  %P3 = getelementptr i8, i8 addrspace(1)* %P2, i32 2
+
+  %A = load i8, i8 addrspace(1)* %P3
+  ret i8 %A
+}


_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160929/4ab36256/attachment.html>