Fix known zero bits for addrspacecast

[yaxunl]

Summary

Currently LLVM assumes that a pointer addrspacecasted to a different addr space is equivalent to trunc or zext bitwise, which is not true. For example, in amdgcn target, when a null pointer is addrspacecasted from addr space 4 to 0, its value is changed from i64 0 to i32 -1.

This patch teaches LLVM not to assume known bits of addrspacecast instruction to its operand.

[yaxunl]

Simplify the test.

[arsenm]

lib/Analysis/ValueTracking.cpp

1028–1029 ↗

(On Diff #78373)

AddrSpaceCast should probably just be removed from the switch

[yaxunl]

lib/Analysis/ValueTracking.cpp

1028–1029 ↗

(On Diff #78373)

Some optimizations will be lost if it is removed.

[arsenm]

lib/Analysis/ValueTracking.cpp

1028–1029 ↗

(On Diff #78373)

Those optimizations are also broken

[yaxunl]

lib/Analysis/ValueTracking.cpp

1028–1029 ↗

(On Diff #78373)

But you are right. We cannot assume anything about the bits after addrspacecast.

[yaxunl]

we are going to lose alignment info after addrspacecast, e.g., the following test will fail:

; CHECK-LABEL: @static_hem_addrspacecast(
; CHECK: , align 16
define <2 x i64> @static_hem_addrspacecast() {
  %t = getelementptr <2 x i64>, <2 x i64>* @x, i32 7
  %t.asc = addrspacecast <2 x i64>* %t to <2 x i64> addrspace(1)*
  %tmp1 = load <2 x i64>, <2 x i64> addrspace(1)* %t.asc, align 1
  ret <2 x i64> %tmp1
}

I guess that's a price we need to pay for correctness.

[mcrosier]

test/Analysis/ValueTracking/knownzero-addrspacecast.ll

25 ↗

(On Diff #78404)

Please add a newline.

[arsenm]

LGTM. I think it should be ok to preserve the alignment though

[yaxunl]

In D26803#599068, @arsenm wrote:

LGTM. I think it should be ok to preserve the alignment though

I think we may need another patch for that. Basically we may need a dedicated parameter for the known zero bits from alignment and pass it through addrspacecast.

test/Analysis/ValueTracking/knownzero-addrspacecast.ll

25 ↗

(On Diff #78404)

will do when committing.

llvm/trunk/lib/Analysis/ValueTracking.cpp

static void computeKnownBitsFromOperator(const Operator *I, APInt &KnownZero,
   case Instruction::FPExt:
   case Instruction::FPToUI:
   case Instruction::FPToSI:
   case Instruction::SIToFP:
   case Instruction::UIToFP:
     break; // Can't work with floating point.
   case Instruction::PtrToInt:
   case Instruction::IntToPtr:
-  case Instruction::AddrSpaceCast: // Pointers could be different sizes.
     // Fall through and handle them the same as zext/trunc.
     LLVM_FALLTHROUGH;
   case Instruction::ZExt:
   case Instruction::Trunc: {
     Type *SrcTy = I->getOperand(0)->getType();
 
     unsigned SrcBitWidth;
     // Note that we handle pointer operands here because of inttoptr/ptrtoint

llvm/trunk/test/Analysis/ValueTracking/knownzero-addrspacecast.ll

+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+; When a pointer is addrspacecasted to a another addr space, we cannot assume
+; anything about the new bits.
+
+target datalayout = "p:32:32-p3:32:32-p4:64:64"
+
+; CHECK-LABEL: @test_shift
+; CHECK-NOT: ret i64 0
+define i64 @test_shift(i8* %p) {
+  %g = addrspacecast i8* %p to i8 addrspace(4)*
+  %i = ptrtoint i8 addrspace(4)* %g to i64
+  %shift = lshr i64 %i, 32
+  ret i64 %shift
+}
+
+; CHECK-LABEL: @test_null
+; A null pointer casted to another addr space may no longer have null value.
+; CHECK-NOT: ret i32 0
+define i32 @test_null() {
+  %g = addrspacecast i8* null to i8 addrspace(3)*
+  %i = ptrtoint i8 addrspace(3)* %g to i32
+  ret i32 %i
+}

llvm/trunk/test/Transforms/InstCombine/loadstore-alignment.ll

 ; RUN: opt -instcombine -S < %s | FileCheck %s
 target datalayout = "E-p:64:64:64-p1:64:64:64-p2:32:32:32-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
 
 @x = external global <2 x i64>, align 16
 @xx = external global [13 x <2 x i64>], align 16
 
 @x.as2 = external addrspace(2) global <2 x i64>, align 16
 
 ; CHECK-LABEL: @static_hem(
 ; CHECK: , align 16
 define <2 x i64> @static_hem() {
   %t = getelementptr <2 x i64>, <2 x i64>* @x, i32 7
   %tmp1 = load <2 x i64>, <2 x i64>* %t, align 1
   ret <2 x i64> %tmp1
 }
 
-; CHECK-LABEL: @static_hem_addrspacecast(
-; CHECK: , align 16
-define <2 x i64> @static_hem_addrspacecast() {
-  %t = getelementptr <2 x i64>, <2 x i64>* @x, i32 7
-  %t.asc = addrspacecast <2 x i64>* %t to <2 x i64> addrspace(1)*
-  %tmp1 = load <2 x i64>, <2 x i64> addrspace(1)* %t.asc, align 1
-  ret <2 x i64> %tmp1
-}
-
-; CHECK-LABEL: @static_hem_addrspacecast_smaller_ptr(
-; CHECK: , align 16
-define <2 x i64> @static_hem_addrspacecast_smaller_ptr() {
-  %t = getelementptr <2 x i64>, <2 x i64>* @x, i32 7
-  %t.asc = addrspacecast <2 x i64>* %t to <2 x i64> addrspace(2)*
-  %tmp1 = load <2 x i64>, <2 x i64> addrspace(2)* %t.asc, align 1
-  ret <2 x i64> %tmp1
-}
-
-; CHECK-LABEL: @static_hem_addrspacecast_larger_ptr(
-; CHECK: , align 16
-define <2 x i64> @static_hem_addrspacecast_larger_ptr() {
-  %t = getelementptr <2 x i64>, <2 x i64> addrspace(2)* @x.as2, i32 7
-  %t.asc = addrspacecast <2 x i64> addrspace(2)* %t to <2 x i64> addrspace(1)*
-  %tmp1 = load <2 x i64>, <2 x i64> addrspace(1)* %t.asc, align 1
-  ret <2 x i64> %tmp1
-}
-
 ; CHECK-LABEL: @hem(
 ; CHECK: , align 16
 define <2 x i64> @hem(i32 %i) {
   %t = getelementptr <2 x i64>, <2 x i64>* @x, i32 %i
   %tmp1 = load <2 x i64>, <2 x i64>* %t, align 1
   ret <2 x i64> %tmp1
 }