Fix constant folding of addrspacecast

[arsenm]

ping

[majnemer]

lib/Analysis/ConstantFolding.cpp

1232–1246 ↗

(On Diff #56841)

This is suspicious to me. It implies that performing a ConstantExpr::getCast(Instruction::AddrSpaceCast) on an IntToPtr does the wrong thing and suggests to me that the bug lies elsewhere.

[arsenm]

Add test that shows ConstantExpr::getAddrSpaceCast does the right thing

[majnemer]

Er, I'm still very confused here. Let's take your example @constant_fold_inttoptr.

trunk turns this into:

define void @constant_fold_inttoptr() {
  store i32 7, i32 addrspace(4)* addrspacecast (i32 addrspace(3)* inttoptr (i32 -1 to i32 addrspace(3)*) to i32 addrspace(4)*)
  ret void
}

I'm not seeing an argument as to why this is not desirable. The target should still have all the information it needs, the only difference I see is that contains an additional ConstantExpr instead of an additional Instruction.

[arsenm]

In D20144#436115, @majnemer wrote:

Er, I'm still very confused here. Let's take your example @constant_fold_inttoptr.

trunk turns this into:

define void @constant_fold_inttoptr() {
  store i32 7, i32 addrspace(4)* addrspacecast (i32 addrspace(3)* inttoptr (i32 -1 to i32 addrspace(3)*) to i32 addrspace(4)*)
  ret void
}

I'm not seeing an argument as to why this is not desirable. The target should still have all the information it needs, the only difference I see is that contains an additional ConstantExpr instead of an additional Instruction.

The constant value for a pointer in one address space may not match the value in another address space, so constant folding it to the same constant is incorrect

[arsenm]

Only need to handle null

[majnemer]

LGTM

[arsenm]

r270293

lib/IR/ConstantFold.cpp

if (isa<UndefValue>(V)) {
     // sext(undef) = 0, because the top bits will all be the same.
     // [us]itofp(undef) = 0, because the result value is bounded.
     if (opc == Instruction::ZExt || opc == Instruction::SExt ||
         opc == Instruction::UIToFP || opc == Instruction::SIToFP)
       return Constant::getNullValue(DestTy);
     return UndefValue::get(DestTy);
   }
 
-  if (V->isNullValue() && !DestTy->isX86_MMXTy())
+  if (V->isNullValue() && !DestTy->isX86_MMXTy() &&
+      opc != Instruction::AddrSpaceCast)
     return Constant::getNullValue(DestTy);
 
   // If the cast operand is a constant expression, there's a few things we can
   // do to try to simplify it.
   if (ConstantExpr *CE = dyn_cast<ConstantExpr>(V)) {
     if (CE->isCast()) {
       // Try hard to fold cast of cast because they are often eliminable.
       if (unsigned newOpc = foldConstantCastPair(opc, CE, DestTy))

test/Assembler/ConstantExprFoldCast.ll

 ; This test checks to make sure that constant exprs fold in some simple situations
 
-; RUN: llvm-as < %s | llvm-dis | not grep cast
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
 ; RUN: verify-uselistorder %s
 
+; CHECK-NOT: bitcast
+; CHECK-NOT: trunc
+; CHECK: addrspacecast
+; CHECK: addrspacecast
+
 @A = global i32* bitcast (i8* null to i32*)  ; Cast null -> fold
 @B = global i32** bitcast (i32** @A to i32**)   ; Cast to same type -> fold
 @C = global i32 trunc (i64 42 to i32)        ; Integral casts
 @D = global i32* bitcast(float*  bitcast (i32* @C to float*) to i32*)  ; cast of cast ptr->ptr
 @E = global i32 ptrtoint(float* inttoptr (i8 5 to float*) to i32)  ; i32 -> ptr -> i32
 
 ; Test folding of binary instrs
 @F = global i32* inttoptr (i32 add (i32 5, i32 -5) to i32*)
 @G = global i32* inttoptr (i32 sub (i32 5, i32 5) to i32*)
 
 ; Address space cast AS0 null-> AS1 null
 @H = global i32 addrspace(1)* addrspacecast(i32* null to i32 addrspace(1)*)
 
+; Address space cast AS1 null-> AS0 null
+@I = global i32* addrspacecast(i32 addrspace(1)* null to i32*)
+
 ; Bitcast -> GEP
-@I = external global { i32 }
-@J = global i32* bitcast ({ i32 }* @I to i32*)
+@J = external global { i32 }
+@K = global i32* bitcast ({ i32 }* @J to i32*)

test/Transforms/InstCombine/addrspacecast.ll

loop.body:
   %i.inc = add i32 %i, 1
   %cmp = icmp ne i32 %i, 48
   br i1 %cmp, label %loop.body, label %end
 
 end:
   ret i32 %sum.inc
 }
 
+; CHECK-LABEL: @constant_fold_null(
+; CHECK: i32 addrspace(3)* null to i32 addrspace(4)*
+define void @constant_fold_null() #0 {
+  %cast = addrspacecast i32 addrspace(3)* null to i32 addrspace(4)*
+  store i32 7, i32 addrspace(4)* %cast
+  ret void
+}
+
+; CHECK-LABEL: @constant_fold_undef(
+; CHECK: ret i32 addrspace(4)* undef
+define i32 addrspace(4)* @constant_fold_undef() #0 {
+  %cast = addrspacecast i32 addrspace(3)* undef to i32 addrspace(4)*
+  ret i32 addrspace(4)* %cast
+}
+
+; CHECK-LABEL: @constant_fold_null_vector(
+; CHECK: addrspacecast (<4 x i32 addrspace(3)*> zeroinitializer to <4 x i32 addrspace(4)*>)
+define <4 x i32 addrspace(4)*> @constant_fold_null_vector() #0 {
+  %cast = addrspacecast <4 x i32 addrspace(3)*> zeroinitializer to <4 x i32 addrspace(4)*>
+  ret <4 x i32 addrspace(4)*> %cast
+}
+
+; CHECK-LABEL: @constant_fold_inttoptr(
+; CHECK: addrspacecast (i32 addrspace(3)* inttoptr (i32 -1 to i32 addrspace(3)*) to i32 addrspace(4)*)
+define void @constant_fold_inttoptr() #0 {
+  %cast = addrspacecast i32 addrspace(3)* inttoptr (i32 -1 to i32 addrspace(3)*) to i32 addrspace(4)*
+  store i32 7, i32 addrspace(4)* %cast
+  ret void
+}
+
+; CHECK-LABEL: @constant_fold_gep_inttoptr(
+; CHECK: addrspacecast (i32 addrspace(3)* inttoptr (i64 1274 to i32 addrspace(3)*) to i32 addrspace(4)*)
+define void @constant_fold_gep_inttoptr() #0 {
+  %k = inttoptr i32 1234 to i32 addrspace(3)*
+  %gep = getelementptr i32, i32 addrspace(3)* %k, i32 10
+  %cast = addrspacecast i32 addrspace(3)* %gep to i32 addrspace(4)*
+  store i32 7, i32 addrspace(4)* %cast
+  ret void
+}

unittests/IR/ConstantsTest.cpp

TEST(ConstantsTest, PointerCast) {
   EXPECT_EQ(Constant::getNullValue(Int64VecTy),
             ConstantExpr::getPointerCast(
               Constant::getNullValue(Int8PtrVecTy), Int64VecTy));
 
   // bitcast <4 x i8*> to <4 x i32*>
   EXPECT_EQ(Constant::getNullValue(Int32PtrVecTy),
             ConstantExpr::getPointerCast(
               Constant::getNullValue(Int8PtrVecTy), Int32PtrVecTy));
+
+  Type *Int32Ptr1Ty = Type::getInt32PtrTy(C, 1);
+  ConstantInt *K = ConstantInt::get(Type::getInt64Ty(C), 1234);
+
+  // Make sure that addrspacecast of inttoptr is not folded away.
+  EXPECT_NE(K,
+            ConstantExpr::getAddrSpaceCast(
+              ConstantExpr::getIntToPtr(K, Int32PtrTy), Int32Ptr1Ty));
+  EXPECT_NE(K,
+            ConstantExpr::getAddrSpaceCast(
+              ConstantExpr::getIntToPtr(K, Int32Ptr1Ty), Int32PtrTy));
+
+  Constant *NullInt32Ptr0 = Constant::getNullValue(Int32PtrTy);
+  Constant *NullInt32Ptr1 = Constant::getNullValue(Int32Ptr1Ty);
+
+  // Make sure that addrspacecast of null is not folded away.
+  EXPECT_NE(Constant::getNullValue(Int32PtrTy),
+            ConstantExpr::getAddrSpaceCast(NullInt32Ptr0, Int32Ptr1Ty));
+
+  EXPECT_NE(Constant::getNullValue(Int32Ptr1Ty),
+            ConstantExpr::getAddrSpaceCast(NullInt32Ptr1, Int32PtrTy));
 }
 
 #define CHECK(x, y) {                                             \
     std::string __s;                                              \
     raw_string_ostream __o(__s);                                  \
     Instruction *__I = cast<ConstantExpr>(x)->getAsInstruction();  \
     __I->print(__o);                 \
     delete __I;               \