Skip to content

[LLVM][IR] Refactor ConstantFold:FoldBitCast to fully support vector ConstantInt/FP. #116787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

paulwalker-arm
Copy link
Collaborator

This fixes the code quality issue reported in #111149.

@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:ir llvm:transforms labels Nov 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 19, 2024

@llvm/pr-subscribers-llvm-ir

Author: Paul Walker (paulwalker-arm)

Changes

This fixes the code quality issue reported in #111149.


Full diff: https://github.com/llvm/llvm-project/pull/116787.diff

3 Files Affected:

  • (modified) llvm/lib/IR/ConstantFold.cpp (+24-23)
  • (modified) llvm/test/Transforms/InstCombine/bitcast.ll (+1)
  • (modified) llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll (+6-13)
diff --git a/llvm/lib/IR/ConstantFold.cpp b/llvm/lib/IR/ConstantFold.cpp
index 2dbc6785c08b9d..3a04f81315e6c6 100644
--- a/llvm/lib/IR/ConstantFold.cpp
+++ b/llvm/lib/IR/ConstantFold.cpp
@@ -71,50 +71,51 @@ static Constant *FoldBitCast(Constant *V, Type *DestTy) {
   if (SrcTy == DestTy)
     return V; // no-op cast
 
-  // Handle casts from one vector constant to another.  We know that the src
-  // and dest type have the same size (otherwise its an illegal cast).
-  if (VectorType *DestPTy = dyn_cast<VectorType>(DestTy)) {
-    if (V->isAllOnesValue())
-      return Constant::getAllOnesValue(DestTy);
+  if (V->isAllOnesValue())
+    return Constant::getAllOnesValue(DestTy);
 
+  // Handle ConstantInt -> ConstantFP
+  if (ConstantInt *CI = dyn_cast<ConstantInt>(V)) {
     // Canonicalize scalar-to-vector bitcasts into vector-to-vector bitcasts
     // This allows for other simplifications (although some of them
     // can only be handled by Analysis/ConstantFolding.cpp).
-    if (!isa<VectorType>(SrcTy))
-      if (isa<ConstantInt>(V) || isa<ConstantFP>(V))
-        return ConstantExpr::getBitCast(ConstantVector::get(V), DestPTy);
-    return nullptr;
-  }
+    if (isa<VectorType>(DestTy) && !isa<VectorType>(SrcTy))
+      return ConstantExpr::getBitCast(ConstantVector::get(V), DestTy);
 
-  // Handle integral constant input.
-  if (ConstantInt *CI = dyn_cast<ConstantInt>(V)) {
+    // Make sure dest type is compatible with the folded fp constant.
     // See note below regarding the PPC_FP128 restriction.
-    if (DestTy->isFloatingPointTy() && !DestTy->isPPC_FP128Ty())
-      return ConstantFP::get(DestTy->getContext(),
-                             APFloat(DestTy->getFltSemantics(),
-                                     CI->getValue()));
+    if (!DestTy->isFPOrFPVectorTy() || DestTy->isPPC_FP128Ty() ||
+        DestTy->getScalarSizeInBits() != SrcTy->getScalarSizeInBits())
+      return nullptr;
 
-    // Otherwise, can't fold this (vector?)
-    return nullptr;
+    return ConstantFP::get(
+        DestTy,
+        APFloat(DestTy->getScalarType()->getFltSemantics(), CI->getValue()));
   }
 
-  // Handle ConstantFP input: FP -> Integral.
+  // Handle ConstantFP -> ConstantInt
   if (ConstantFP *FP = dyn_cast<ConstantFP>(V)) {
+    // Canonicalize scalar-to-vector bitcasts into vector-to-vector bitcasts
+    // This allows for other simplifications (although some of them
+    // can only be handled by Analysis/ConstantFolding.cpp).
+    if (isa<VectorType>(DestTy) && !isa<VectorType>(SrcTy))
+      return ConstantExpr::getBitCast(ConstantVector::get(V), DestTy);
+
     // PPC_FP128 is really the sum of two consecutive doubles, where the first
     // double is always stored first in memory, regardless of the target
     // endianness. The memory layout of i128, however, depends on the target
     // endianness, and so we can't fold this without target endianness
     // information. This should instead be handled by
     // Analysis/ConstantFolding.cpp
-    if (FP->getType()->isPPC_FP128Ty())
+    if (SrcTy->isPPC_FP128Ty())
       return nullptr;
 
     // Make sure dest type is compatible with the folded integer constant.
-    if (!DestTy->isIntegerTy())
+    if (!DestTy->isIntOrIntVectorTy() ||
+        DestTy->getScalarSizeInBits() != SrcTy->getScalarSizeInBits())
       return nullptr;
 
-    return ConstantInt::get(FP->getContext(),
-                            FP->getValueAPF().bitcastToAPInt());
+    return ConstantInt::get(DestTy, FP->getValueAPF().bitcastToAPInt());
   }
 
   return nullptr;
diff --git a/llvm/test/Transforms/InstCombine/bitcast.ll b/llvm/test/Transforms/InstCombine/bitcast.ll
index ce5a4635a9b0a2..37d41de3e99911 100644
--- a/llvm/test/Transforms/InstCombine/bitcast.ll
+++ b/llvm/test/Transforms/InstCombine/bitcast.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -use-constant-fp-for-fixed-length-splat -use-constant-int-for-fixed-length-splat -S | FileCheck %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 target triple = "x86_64-apple-darwin10.0.0"
diff --git a/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll b/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll
index b475b8199541d5..a3ad5f01391422 100644
--- a/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll
+++ b/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll
@@ -1,6 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -passes=instsimplify -S | FileCheck %s --check-prefixes=CHECK,CONSTVEC
-; RUN: opt < %s -passes=instsimplify -use-constant-fp-for-fixed-length-splat -use-constant-int-for-fixed-length-splat -S | FileCheck %s --check-prefixes=CHECK,CONSTSPLAT
+; RUN: opt < %s -passes=instsimplify -S | FileCheck %s
+; RUN: opt < %s -passes=instsimplify -use-constant-fp-for-fixed-length-splat -use-constant-int-for-fixed-length-splat -S | FileCheck %s
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-f64:32:64-v64:64:64-v128:128:128"
 
 define <2 x i64> @test1() {
@@ -76,12 +76,8 @@ define <4 x i32> @test9(<1 x i64> %y) {
 }
 
 define <1 x i1> @test10() {
-; CONSTVEC-LABEL: @test10(
-; CONSTVEC-NEXT:    [[RET:%.*]] = icmp eq <1 x i64> <i64 bitcast (<1 x double> splat (double 0xFFFFFFFFFFFFFFFF) to i64)>, zeroinitializer
-; CONSTVEC-NEXT:    ret <1 x i1> [[RET]]
-;
-; CONSTSPLAT-LABEL: @test10(
-; CONSTSPLAT-NEXT:    ret <1 x i1> zeroinitializer
+; CHECK-LABEL: @test10(
+; CHECK-NEXT:    ret <1 x i1> zeroinitializer
 ;
   %ret = icmp eq <1 x i64> <i64 bitcast (<1 x double> <double 0xFFFFFFFFFFFFFFFF> to i64)>, zeroinitializer
   ret <1 x i1> %ret
@@ -282,11 +278,8 @@ define <16 x i8> @bitcast_constexpr_16i8_8i16_u256uuu256uu() {
 }
 
 define <1 x i32> @bitcast_constexpr_scalar_fp_to_vector_int() {
-; CONSTVEC-LABEL: @bitcast_constexpr_scalar_fp_to_vector_int(
-; CONSTVEC-NEXT:    ret <1 x i32> splat (i32 1065353216)
-;
-; CONSTSPLAT-LABEL: @bitcast_constexpr_scalar_fp_to_vector_int(
-; CONSTSPLAT-NEXT:    ret <1 x i32> bitcast (<1 x float> splat (float 1.000000e+00) to <1 x i32>)
+; CHECK-LABEL: @bitcast_constexpr_scalar_fp_to_vector_int(
+; CHECK-NEXT:    ret <1 x i32> splat (i32 1065353216)
 ;
   %res = bitcast float 1.0 to <1 x i32>
   ret <1 x i32> %res

@llvmbot
Copy link
Member

llvmbot commented Nov 19, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Paul Walker (paulwalker-arm)

Changes

This fixes the code quality issue reported in #111149.


Full diff: https://github.com/llvm/llvm-project/pull/116787.diff

3 Files Affected:

  • (modified) llvm/lib/IR/ConstantFold.cpp (+24-23)
  • (modified) llvm/test/Transforms/InstCombine/bitcast.ll (+1)
  • (modified) llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll (+6-13)
diff --git a/llvm/lib/IR/ConstantFold.cpp b/llvm/lib/IR/ConstantFold.cpp
index 2dbc6785c08b9d..3a04f81315e6c6 100644
--- a/llvm/lib/IR/ConstantFold.cpp
+++ b/llvm/lib/IR/ConstantFold.cpp
@@ -71,50 +71,51 @@ static Constant *FoldBitCast(Constant *V, Type *DestTy) {
   if (SrcTy == DestTy)
     return V; // no-op cast
 
-  // Handle casts from one vector constant to another.  We know that the src
-  // and dest type have the same size (otherwise its an illegal cast).
-  if (VectorType *DestPTy = dyn_cast<VectorType>(DestTy)) {
-    if (V->isAllOnesValue())
-      return Constant::getAllOnesValue(DestTy);
+  if (V->isAllOnesValue())
+    return Constant::getAllOnesValue(DestTy);
 
+  // Handle ConstantInt -> ConstantFP
+  if (ConstantInt *CI = dyn_cast<ConstantInt>(V)) {
     // Canonicalize scalar-to-vector bitcasts into vector-to-vector bitcasts
     // This allows for other simplifications (although some of them
     // can only be handled by Analysis/ConstantFolding.cpp).
-    if (!isa<VectorType>(SrcTy))
-      if (isa<ConstantInt>(V) || isa<ConstantFP>(V))
-        return ConstantExpr::getBitCast(ConstantVector::get(V), DestPTy);
-    return nullptr;
-  }
+    if (isa<VectorType>(DestTy) && !isa<VectorType>(SrcTy))
+      return ConstantExpr::getBitCast(ConstantVector::get(V), DestTy);
 
-  // Handle integral constant input.
-  if (ConstantInt *CI = dyn_cast<ConstantInt>(V)) {
+    // Make sure dest type is compatible with the folded fp constant.
     // See note below regarding the PPC_FP128 restriction.
-    if (DestTy->isFloatingPointTy() && !DestTy->isPPC_FP128Ty())
-      return ConstantFP::get(DestTy->getContext(),
-                             APFloat(DestTy->getFltSemantics(),
-                                     CI->getValue()));
+    if (!DestTy->isFPOrFPVectorTy() || DestTy->isPPC_FP128Ty() ||
+        DestTy->getScalarSizeInBits() != SrcTy->getScalarSizeInBits())
+      return nullptr;
 
-    // Otherwise, can't fold this (vector?)
-    return nullptr;
+    return ConstantFP::get(
+        DestTy,
+        APFloat(DestTy->getScalarType()->getFltSemantics(), CI->getValue()));
   }
 
-  // Handle ConstantFP input: FP -> Integral.
+  // Handle ConstantFP -> ConstantInt
   if (ConstantFP *FP = dyn_cast<ConstantFP>(V)) {
+    // Canonicalize scalar-to-vector bitcasts into vector-to-vector bitcasts
+    // This allows for other simplifications (although some of them
+    // can only be handled by Analysis/ConstantFolding.cpp).
+    if (isa<VectorType>(DestTy) && !isa<VectorType>(SrcTy))
+      return ConstantExpr::getBitCast(ConstantVector::get(V), DestTy);
+
     // PPC_FP128 is really the sum of two consecutive doubles, where the first
     // double is always stored first in memory, regardless of the target
     // endianness. The memory layout of i128, however, depends on the target
     // endianness, and so we can't fold this without target endianness
     // information. This should instead be handled by
     // Analysis/ConstantFolding.cpp
-    if (FP->getType()->isPPC_FP128Ty())
+    if (SrcTy->isPPC_FP128Ty())
       return nullptr;
 
     // Make sure dest type is compatible with the folded integer constant.
-    if (!DestTy->isIntegerTy())
+    if (!DestTy->isIntOrIntVectorTy() ||
+        DestTy->getScalarSizeInBits() != SrcTy->getScalarSizeInBits())
       return nullptr;
 
-    return ConstantInt::get(FP->getContext(),
-                            FP->getValueAPF().bitcastToAPInt());
+    return ConstantInt::get(DestTy, FP->getValueAPF().bitcastToAPInt());
   }
 
   return nullptr;
diff --git a/llvm/test/Transforms/InstCombine/bitcast.ll b/llvm/test/Transforms/InstCombine/bitcast.ll
index ce5a4635a9b0a2..37d41de3e99911 100644
--- a/llvm/test/Transforms/InstCombine/bitcast.ll
+++ b/llvm/test/Transforms/InstCombine/bitcast.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -use-constant-fp-for-fixed-length-splat -use-constant-int-for-fixed-length-splat -S | FileCheck %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 target triple = "x86_64-apple-darwin10.0.0"
diff --git a/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll b/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll
index b475b8199541d5..a3ad5f01391422 100644
--- a/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll
+++ b/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll
@@ -1,6 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -passes=instsimplify -S | FileCheck %s --check-prefixes=CHECK,CONSTVEC
-; RUN: opt < %s -passes=instsimplify -use-constant-fp-for-fixed-length-splat -use-constant-int-for-fixed-length-splat -S | FileCheck %s --check-prefixes=CHECK,CONSTSPLAT
+; RUN: opt < %s -passes=instsimplify -S | FileCheck %s
+; RUN: opt < %s -passes=instsimplify -use-constant-fp-for-fixed-length-splat -use-constant-int-for-fixed-length-splat -S | FileCheck %s
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-f64:32:64-v64:64:64-v128:128:128"
 
 define <2 x i64> @test1() {
@@ -76,12 +76,8 @@ define <4 x i32> @test9(<1 x i64> %y) {
 }
 
 define <1 x i1> @test10() {
-; CONSTVEC-LABEL: @test10(
-; CONSTVEC-NEXT:    [[RET:%.*]] = icmp eq <1 x i64> <i64 bitcast (<1 x double> splat (double 0xFFFFFFFFFFFFFFFF) to i64)>, zeroinitializer
-; CONSTVEC-NEXT:    ret <1 x i1> [[RET]]
-;
-; CONSTSPLAT-LABEL: @test10(
-; CONSTSPLAT-NEXT:    ret <1 x i1> zeroinitializer
+; CHECK-LABEL: @test10(
+; CHECK-NEXT:    ret <1 x i1> zeroinitializer
 ;
   %ret = icmp eq <1 x i64> <i64 bitcast (<1 x double> <double 0xFFFFFFFFFFFFFFFF> to i64)>, zeroinitializer
   ret <1 x i1> %ret
@@ -282,11 +278,8 @@ define <16 x i8> @bitcast_constexpr_16i8_8i16_u256uuu256uu() {
 }
 
 define <1 x i32> @bitcast_constexpr_scalar_fp_to_vector_int() {
-; CONSTVEC-LABEL: @bitcast_constexpr_scalar_fp_to_vector_int(
-; CONSTVEC-NEXT:    ret <1 x i32> splat (i32 1065353216)
-;
-; CONSTSPLAT-LABEL: @bitcast_constexpr_scalar_fp_to_vector_int(
-; CONSTSPLAT-NEXT:    ret <1 x i32> bitcast (<1 x float> splat (float 1.000000e+00) to <1 x i32>)
+; CHECK-LABEL: @bitcast_constexpr_scalar_fp_to_vector_int(
+; CHECK-NEXT:    ret <1 x i32> splat (i32 1065353216)
 ;
   %res = bitcast float 1.0 to <1 x i32>
   ret <1 x i32> %res

@paulwalker-arm paulwalker-arm changed the title [LLVM][IR] Refactor FoldBitCast to fully support vector ConstantInt/FP. [LLVM][IR] Refactor ConstantFold:FoldBitCast to fully support vector ConstantInt/FP. Nov 19, 2024
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@paulwalker-arm paulwalker-arm merged commit af641ff into llvm:main Nov 21, 2024
12 checks passed
@paulwalker-arm paulwalker-arm deleted the vector-constants-ir-bitcasts branch November 21, 2024 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:ir llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants