[IR] Mark zext/sext constant expressions as undesirable

Introduce isDesirableCastOp() which determines whether IR builder and constant folding should produce constant expressions for a given cast type. This mirrors what we do for binary operators. Mark zext/sext as undesirable, which prevents most creations of such constant expressions. This is still somewhat incomplete and there are a few more places that can create zext/sext expressions. This is part of the work for https://discourse.llvm.org/t/rfc-remove-most-constant-expressions/63179. The reason for the odd result in the constantexpr-fneg.c test is that initially the "a[]" global is created with an [0 x i32] type, at which point the icmp expression cannot be folded. Later it is replaced with an [1 x i32] global and the icmp gets folded away. But at that point we no longer fold the zext.
llvm · Oct 2, 2023 · 3b25407 · 3b25407
1 parent 2214026
commit 3b25407
Show file tree

Hide file tree

Showing 14 changed files with 158 additions and 101 deletions.
diff --git a/clang/test/CodeGen/constantexpr-fneg.c b/clang/test/CodeGen/constantexpr-fneg.c
@@ -8,9 +8,11 @@
 // CHECK:      entry:
 // CHECK-NEXT:   %retval = alloca i32
 // CHECK-NEXT:   store i32 0, ptr %retval
+// CHECK-NEXT:   [[ZEXT:%.*]] = zext i1 true to i32
+// CHECK-NEXT:   [[SITOFP:%.*]] = sitofp i32 [[ZEXT]] to float
 // CHECK-NEXT:   [[LV:%.*]] = load ptr, ptr @c
-// CHECK-NEXT:   store float 1.000000e+00, ptr [[LV]], align 4
-// CHECK-NEXT:   [[FNEG:%.*]] = fneg float 1.000000e+00
+// CHECK-NEXT:   store float [[SITOFP]], ptr [[LV]], align 4
+// CHECK-NEXT:   [[FNEG:%.*]] = fneg float [[SITOFP]]
 // CHECK-NEXT:   [[CONV:%.*]] = fptosi float [[FNEG]] to i32
 // CHECK-NEXT:   ret i32 [[CONV]]
 

diff --git a/clang/test/CodeGenCXX/weak-external.cpp b/clang/test/CodeGenCXX/weak-external.cpp
@@ -80,19 +80,23 @@ namespace not_weak_on_first {
 namespace constant_eval {
   [[gnu::weak]] extern int a;
   // CHECK-LABEL: define {{.*}} @__cxx_global_var_init
-  // CHECK:     store i8 zext (i1 icmp ne (ptr @_ZN13constant_eval1aE, ptr null) to i8), ptr @_ZN13constant_eval6has_a1E,
+  // CHECK:     [[ZEXT:%.*]] = zext i1 icmp ne (ptr @_ZN13constant_eval1aE, ptr null) to i8
+  // CHECK:     store i8 [[ZEXT]], ptr @_ZN13constant_eval6has_a1E,
   bool has_a1 = &a;
   // CHECK-LABEL: define {{.*}} @__cxx_global_var_init
-  // CHECK:     store i8 zext (i1 icmp ne (ptr @_ZN13constant_eval1aE, ptr null) to i8), ptr @_ZN13constant_eval6has_a2E,
+  // CHECK:     [[ZEXT:%.*]] = zext i1 icmp ne (ptr @_ZN13constant_eval1aE, ptr null) to i8
+  // CHECK:     store i8 [[ZEXT]], ptr @_ZN13constant_eval6has_a2E,
   bool has_a2 = &a != nullptr;
 
   struct X {
     [[gnu::weak]] void f();
   };
   // CHECK-LABEL: define {{.*}} @__cxx_global_var_init
-  // CHECK:     store i8 zext (i1 icmp ne (i{{32|64}} ptrtoint (ptr @_ZN13constant_eval1X1fEv to i{{32|64}}), i{{32|64}} 0) to i8), ptr @_ZN13constant_eval6has_f1E,
+  // CHECK:     [[ZEXT:%.*]] = zext i1 icmp ne (i{{32|64}} ptrtoint (ptr @_ZN13constant_eval1X1fEv to i{{32|64}}), i{{32|64}} 0) to i8
+  // CHECK:     store i8 [[ZEXT]], ptr @_ZN13constant_eval6has_f1E,
   bool has_f1 = &X::f;
   // CHECK-LABEL: define {{.*}} @__cxx_global_var_init
-  // CHECK:     store i8 zext (i1 icmp ne (i{{32|64}} ptrtoint (ptr @_ZN13constant_eval1X1fEv to i{{32|64}}), i{{32|64}} 0) to i8), ptr @_ZN13constant_eval6has_f2E,
+  // CHECK:     [[ZEXT:%.*]] = zext i1 icmp ne (i{{32|64}} ptrtoint (ptr @_ZN13constant_eval1X1fEv to i{{32|64}}), i{{32|64}} 0) to i8
+  // CHECK:     store i8 [[ZEXT]], ptr @_ZN13constant_eval6has_f2E,
   bool has_f2 = &X::f != nullptr;
 }
diff --git a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -606,7 +606,8 @@ int test_and_ptr(private char* p1, local char* p2) {
 // NOOPT: store ptr addrspace(1) null, ptr addrspace(5) %glob, align 8
 // NOOPT: %{{.*}} = sub i64 %{{.*}}, 0
 // NOOPT: call void @test_fold_callee
-// NOOPT: %{{.*}} = add nsw i64 %1, sext (i32 ptrtoint (ptr addrspace(5) addrspacecast (ptr null to ptr addrspace(5)) to i32) to i64)
+// NOOPT: %[[SEXT:.*]] = sext i32 ptrtoint (ptr addrspace(5) addrspacecast (ptr null to ptr addrspace(5)) to i32) to i64
+// NOOPT: %{{.*}} = add nsw i64 %1, %[[SEXT]]
 // NOOPT: %{{.*}} = sub nsw i64 %{{.*}}, 1
 void test_fold_callee(void);
 void test_fold_private(void) {
@@ -621,7 +622,8 @@ void test_fold_private(void) {
 // NOOPT: store ptr addrspace(1) null, ptr addrspace(5) %glob, align 8
 // NOOPT: %{{.*}} = sub i64 %{{.*}}, 0
 // NOOPT: call void @test_fold_callee
-// NOOPT: %{{.*}} = add nsw i64 %{{.*}}, sext (i32 ptrtoint (ptr addrspace(3) addrspacecast (ptr null to ptr addrspace(3)) to i32) to i64)
+// NOOPT: %[[SEXT:.*]] = sext i32 ptrtoint (ptr addrspace(3) addrspacecast (ptr null to ptr addrspace(3)) to i32) to i64
+// NOOPT: %{{.*}} = add nsw i64 %{{.*}}, %[[SEXT]]
 // NOOPT: %{{.*}} = sub nsw i64 %{{.*}}, 1
 void test_fold_local(void) {
   global int* glob = (test_fold_callee(), (global int*)(generic char*)0);

diff --git a/llvm/include/llvm/Analysis/TargetFolder.h b/llvm/include/llvm/Analysis/TargetFolder.h
@@ -187,7 +187,7 @@ class TargetFolder final : public IRBuilderFolder {
   Value *FoldCast(Instruction::CastOps Op, Value *V,
                   Type *DestTy) const override {
     if (auto *C = dyn_cast<Constant>(V))
-      return Fold(ConstantExpr::getCast(Op, C, DestTy));
+      return ConstantFoldCastOperand(Op, C, DestTy, DL);
     return nullptr;
   }
 

diff --git a/llvm/include/llvm/IR/ConstantFolder.h b/llvm/include/llvm/IR/ConstantFolder.h
@@ -175,8 +175,11 @@ class ConstantFolder final : public IRBuilderFolder {
 
   Value *FoldCast(Instruction::CastOps Op, Value *V,
                   Type *DestTy) const override {
-    if (auto *C = dyn_cast<Constant>(V))
-      return ConstantExpr::getCast(Op, C, DestTy);
+    if (auto *C = dyn_cast<Constant>(V)) {
+      if (ConstantExpr::isDesirableCastOp(Op))
+        return ConstantExpr::getCast(Op, C, DestTy);
+      return ConstantFoldCastInstruction(Op, C, DestTy);
+    }
     return nullptr;
   }
 

diff --git a/llvm/include/llvm/IR/Constants.h b/llvm/include/llvm/IR/Constants.h
@@ -1330,6 +1330,9 @@ class ConstantExpr : public Constant {
   /// supported.
   static bool isSupportedBinOp(unsigned Opcode);
 
+  /// Whether creating a constant expression for this cast is desirable.
+  static bool isDesirableCastOp(unsigned Opcode);
+
   /// Whether creating a constant expression for this getelementptr type is
   /// supported.
   static bool isSupportedGetElementPtr(const Type *SrcElemTy) {

diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -1435,7 +1435,7 @@ Constant *llvm::ConstantFoldCastOperand(unsigned Opcode, Constant *C,
                                             /*IsSigned=*/false);
       }
     }
-    return ConstantExpr::getCast(Opcode, C, DestTy);
+    break;
   case Instruction::IntToPtr:
     // If the input is a ptrtoint, turn the pair into a ptr to ptr bitcast if
     // the int size is >= the ptr size and the address spaces are the same.
@@ -1454,8 +1454,7 @@ Constant *llvm::ConstantFoldCastOperand(unsigned Opcode, Constant *C,
         }
       }
     }
-
-    return ConstantExpr::getCast(Opcode, C, DestTy);
+    break;
   case Instruction::Trunc:
   case Instruction::ZExt:
   case Instruction::SExt:
@@ -1466,10 +1465,14 @@ Constant *llvm::ConstantFoldCastOperand(unsigned Opcode, Constant *C,
   case Instruction::FPToUI:
   case Instruction::FPToSI:
   case Instruction::AddrSpaceCast:
-      return ConstantExpr::getCast(Opcode, C, DestTy);
+    break;
   case Instruction::BitCast:
     return FoldBitCast(C, DestTy, DL);
   }
+
+  if (ConstantExpr::isDesirableCastOp(Opcode))
+    return ConstantExpr::getCast(Opcode, C, DestTy);
+  return ConstantFoldCastInstruction(Opcode, C, DestTy);
 }
 
 //===----------------------------------------------------------------------===//

diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
@@ -2331,6 +2331,28 @@ bool ConstantExpr::isSupportedBinOp(unsigned Opcode) {
   }
 }
 
+bool ConstantExpr::isDesirableCastOp(unsigned Opcode) {
+  switch (Opcode) {
+  case Instruction::ZExt:
+  case Instruction::SExt:
+    return false;
+  case Instruction::Trunc:
+  case Instruction::FPTrunc:
+  case Instruction::FPExt:
+  case Instruction::UIToFP:
+  case Instruction::SIToFP:
+  case Instruction::FPToUI:
+  case Instruction::FPToSI:
+  case Instruction::PtrToInt:
+  case Instruction::IntToPtr:
+  case Instruction::BitCast:
+  case Instruction::AddrSpaceCast:
+    return true;
+  default:
+    llvm_unreachable("Argument must be cast opcode");
+  }
+}
+
 Constant *ConstantExpr::getSizeOf(Type* Ty) {
   // sizeof is implemented as: (i64) gep (Ty*)null, 1
   // Note that a non-inbounds gep is used, as null isn't within any object.

diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -1225,10 +1225,12 @@ void SCCPInstVisitor::visitCastInst(CastInst &I) {
 
   if (Constant *OpC = getConstant(OpSt, I.getOperand(0)->getType())) {
     // Fold the constant as we build.
-    Constant *C = ConstantFoldCastOperand(I.getOpcode(), OpC, I.getType(), DL);
-    markConstant(&I, C);
-  } else if (I.getDestTy()->isIntegerTy() &&
-             I.getSrcTy()->isIntOrIntVectorTy()) {
+    if (Constant *C =
+            ConstantFoldCastOperand(I.getOpcode(), OpC, I.getType(), DL))
+      return (void)markConstant(&I, C);
+  }
+
+  if (I.getDestTy()->isIntegerTy() && I.getSrcTy()->isIntOrIntVectorTy()) {
     auto &LV = getValueState(&I);
     ConstantRange OpRange = getConstantRange(OpSt, I.getSrcTy());
 

diff --git a/llvm/test/Transforms/InstCombine/constant-fold-shifts.ll b/llvm/test/Transforms/InstCombine/constant-fold-shifts.ll
@@ -8,7 +8,7 @@
 define void @ossfuzz_14169_test1(ptr %a0) {
 ; CHECK-LABEL: @ossfuzz_14169_test1(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    store ptr undef, ptr undef, align 8
+; CHECK-NEXT:    store ptr poison, ptr undef, align 8
 ; CHECK-NEXT:    ret void
 ;
 bb:
@@ -24,7 +24,7 @@ bb:
 define void @ossfuzz_14169_test2(ptr %a0) {
 ; CHECK-LABEL: @ossfuzz_14169_test2(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    store ptr undef, ptr undef, align 8
+; CHECK-NEXT:    store ptr poison, ptr undef, align 8
 ; CHECK-NEXT:    ret void
 ;
 bb:

diff --git a/llvm/test/Transforms/InstCombine/gep-custom-dl.ll b/llvm/test/Transforms/InstCombine/gep-custom-dl.ll
@@ -168,7 +168,7 @@ define i32 @test10() {
 define i16 @constant_fold_custom_dl() {
 ; CHECK-LABEL: @constant_fold_custom_dl(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    ret i16 ptrtoint (ptr addrspace(1) getelementptr (i8, ptr addrspace(1) getelementptr inbounds ([1000 x i8], ptr addrspace(1) @X_as1, i32 1, i32 0), i32 sext (i16 sub (i16 0, i16 ptrtoint (ptr addrspace(1) @X_as1 to i16)) to i32)) to i16)
+; CHECK-NEXT:    ret i16 ptrtoint (ptr addrspace(1) getelementptr (i8, ptr addrspace(1) getelementptr inbounds ([1000 x i8], ptr addrspace(1) @X_as1, i32 1, i32 0), i16 sub (i16 0, i16 ptrtoint (ptr addrspace(1) @X_as1 to i16))) to i16)
 ;
 
 entry:

diff --git a/llvm/test/Transforms/InstCombine/hoist-xor-by-constant-from-xor-by-value.ll b/llvm/test/Transforms/InstCombine/hoist-xor-by-constant-from-xor-by-value.ll
@@ -94,9 +94,11 @@ entry:
 
 define i16 @constantexpr2() {
 ; CHECK-LABEL: @constantexpr2(
+; CHECK-NEXT:    [[I1:%.*]] = zext i1 icmp ne (ptr getelementptr inbounds ([6 x [1 x i64]], ptr @global_constant3, i64 0, i64 5, i64 0), ptr @global_constant4) to i16
 ; CHECK-NEXT:    [[I2:%.*]] = load ptr, ptr @global_constant5, align 1
 ; CHECK-NEXT:    [[I3:%.*]] = load i16, ptr [[I2]], align 1
-; CHECK-NEXT:    [[I5:%.*]] = xor i16 [[I3]], xor (i16 zext (i1 icmp ne (ptr getelementptr inbounds ([6 x [1 x i64]], ptr @global_constant3, i64 0, i64 5, i64 0), ptr @global_constant4) to i16), i16 -1)
+; CHECK-NEXT:    [[I4:%.*]] = xor i16 [[I3]], [[I1]]
+; CHECK-NEXT:    [[I5:%.*]] = xor i16 [[I4]], -1
 ; CHECK-NEXT:    ret i16 [[I5]]
 ;
   %i0 = icmp ne ptr getelementptr inbounds ([6 x [1 x i64]], ptr @global_constant3, i16 0, i16 5, i16 0), @global_constant4