[LLVM][ConstFolds] Verify a scalar src before attempting scalar->vector bitcast transformation. #111149

paulwalker-arm · 2024-10-04T13:22:13Z

It was previously safe to assume isa<Constant{Int,FP}> meant a scalar value. This is not true when use-constant-##-for-###-splat are enabled.

…or bitcast transformation. It was previously safe to assume isa<Constant{Int,FP}> meant a scalar value. This is not true when use-constant-##-for-###-splat are enabled.

llvmbot · 2024-10-04T13:22:45Z

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-analysis

Author: Paul Walker (paulwalker-arm)

Changes

It was previously safe to assume isa<Constant{Int,FP}> meant a scalar value. This is not true when use-constant-##-for-###-splat are enabled.

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

3 Files Affected:

(modified) llvm/lib/Analysis/ConstantFolding.cpp (+2-1)
(modified) llvm/lib/IR/ConstantFold.cpp (+3-2)
(modified) llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll (+45-14)

diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp
index a7a6de3f3b97b0..a6ef271da11f1a 100644
--- a/llvm/lib/Analysis/ConstantFolding.cpp
+++ b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -145,7 +145,8 @@ Constant *FoldBitCast(Constant *C, Type *DestTy, const DataLayout &DL) {
 
   // If this is a scalar -> vector cast, convert the input into a <1 x scalar>
   // vector so the code below can handle it uniformly.
-  if (isa<ConstantFP>(C) || isa<ConstantInt>(C)) {
+  if (!isa<VectorType>(C->getType()) &&
+      (isa<ConstantFP>(C) || isa<ConstantInt>(C))) {
     Constant *Ops = C; // don't take the address of C!
     return FoldBitCast(ConstantVector::get(Ops), DestTy, DL);
   }
diff --git a/llvm/lib/IR/ConstantFold.cpp b/llvm/lib/IR/ConstantFold.cpp
index a6f46da313e213..57d9a03c9c22b8 100644
--- a/llvm/lib/IR/ConstantFold.cpp
+++ b/llvm/lib/IR/ConstantFold.cpp
@@ -81,8 +81,9 @@ static Constant *FoldBitCast(Constant *V, Type *DestTy) {
     // 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<ConstantInt>(V) || isa<ConstantFP>(V))
-      return ConstantExpr::getBitCast(ConstantVector::get(V), DestPTy);
+    if (!isa<VectorType>(SrcTy))
+      if (isa<ConstantInt>(V) || isa<ConstantFP>(V))
+        return ConstantExpr::getBitCast(ConstantVector::get(V), DestPTy);
     return nullptr;
   }
 
diff --git a/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll b/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll
index 68ff0859beb2a7..2e75a0d2c98abf 100644
--- a/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll
+++ b/llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -passes=instsimplify -S | FileCheck %s
+; 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
 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() {
@@ -67,17 +68,24 @@ define <4 x i32> @test8(<1 x i64> %y) {
 }
 
 define <4 x i32> @test9(<1 x i64> %y) {
-; CHECK-LABEL: @test9(
-; CHECK-NEXT:    ret <4 x i32> <i32 -1, i32 -1, i32 -1, i32 -1>
+; CONSTVEC-LABEL: @test9(
+; CONSTVEC-NEXT:    ret <4 x i32> <i32 -1, i32 -1, i32 -1, i32 -1>
+;
+; CONSTSPLAT-LABEL: @test9(
+; CONSTSPLAT-NEXT:    ret <4 x i32> splat (i32 -1)
 ;
   %c = bitcast <2 x i64> <i64 -1, i64 -1> to <4 x i32>
   ret <4 x i32> %c
 }
 
 define <1 x i1> @test10() {
-; CHECK-LABEL: @test10(
-; CHECK-NEXT:    [[RET:%.*]] = icmp eq <1 x i64> <i64 bitcast (<1 x double> <double 0xFFFFFFFFFFFFFFFF> to i64)>, zeroinitializer
-; CHECK-NEXT:    ret <1 x i1> [[RET]]
+; CONSTVEC-LABEL: @test10(
+; CONSTVEC-NEXT:    [[RET:%.*]] = icmp eq <1 x i64> <i64 bitcast (<1 x double> <double 0xFFFFFFFFFFFFFFFF> to i64)>, zeroinitializer
+; CONSTVEC-NEXT:    ret <1 x i1> [[RET]]
+;
+; CONSTSPLAT-LABEL: @test10(
+; CONSTSPLAT-NEXT:    [[RET:%.*]] = icmp eq <1 x i64> splat (i64 -1), zeroinitializer
+; CONSTSPLAT-NEXT:    ret <1 x i1> [[RET]]
 ;
   %ret = icmp eq <1 x i64> <i64 bitcast (<1 x double> <double 0xFFFFFFFFFFFFFFFF> to i64)>, zeroinitializer
   ret <1 x i1> %ret
@@ -85,8 +93,11 @@ define <1 x i1> @test10() {
 
 ; from MultiSource/Benchmarks/Bullet
 define <2 x float> @foo() {
-; CHECK-LABEL: @foo(
-; CHECK-NEXT:    ret <2 x float> <float 0xFFFFFFFFE0000000, float 0xFFFFFFFFE0000000>
+; CONSTVEC-LABEL: @foo(
+; CONSTVEC-NEXT:    ret <2 x float> <float 0xFFFFFFFFE0000000, float 0xFFFFFFFFE0000000>
+;
+; CONSTSPLAT-LABEL: @foo(
+; CONSTSPLAT-NEXT:    ret <2 x float> splat (float 0xFFFFFFFFE0000000)
 ;
   %cast = bitcast i64 -1 to <2 x float>
   ret <2 x float> %cast
@@ -94,16 +105,22 @@ define <2 x float> @foo() {
 
 
 define <2 x double> @foo2() {
-; CHECK-LABEL: @foo2(
-; CHECK-NEXT:    ret <2 x double> <double 0xFFFFFFFFFFFFFFFF, double 0xFFFFFFFFFFFFFFFF>
+; CONSTVEC-LABEL: @foo2(
+; CONSTVEC-NEXT:    ret <2 x double> <double 0xFFFFFFFFFFFFFFFF, double 0xFFFFFFFFFFFFFFFF>
+;
+; CONSTSPLAT-LABEL: @foo2(
+; CONSTSPLAT-NEXT:    ret <2 x double> splat (double 0xFFFFFFFFFFFFFFFF)
 ;
   %cast = bitcast i128 -1 to <2 x double>
   ret <2 x double> %cast
 }
 
 define <1 x float> @foo3() {
-; CHECK-LABEL: @foo3(
-; CHECK-NEXT:    ret <1 x float> <float 0xFFFFFFFFE0000000>
+; CONSTVEC-LABEL: @foo3(
+; CONSTVEC-NEXT:    ret <1 x float> <float 0xFFFFFFFFE0000000>
+;
+; CONSTSPLAT-LABEL: @foo3(
+; CONSTSPLAT-NEXT:    ret <1 x float> splat (float 0xFFFFFFFFE0000000)
 ;
   %cast = bitcast i32 -1 to <1 x float>
   ret <1 x float> %cast
@@ -126,8 +143,11 @@ define double @foo5() {
 }
 
 define <2 x double> @foo6() {
-; CHECK-LABEL: @foo6(
-; CHECK-NEXT:    ret <2 x double> <double 0xFFFFFFFFFFFFFFFF, double 0xFFFFFFFFFFFFFFFF>
+; CONSTVEC-LABEL: @foo6(
+; CONSTVEC-NEXT:    ret <2 x double> <double 0xFFFFFFFFFFFFFFFF, double 0xFFFFFFFFFFFFFFFF>
+;
+; CONSTSPLAT-LABEL: @foo6(
+; CONSTSPLAT-NEXT:    ret <2 x double> splat (double 0xFFFFFFFFFFFFFFFF)
 ;
   %cast = bitcast <4 x i32><i32 -1, i32 -1, i32 -1, i32 -1> to <2 x double>
   ret <2 x double> %cast
@@ -276,3 +296,14 @@ define <16 x i8> @bitcast_constexpr_16i8_8i16_u256uuu256uu() {
   %cast = bitcast <8 x i16><i16 undef, i16 256, i16 undef, i16 undef, i16 undef, i16 256, i16 undef, i16 undef> to <16 x i8>
   ret <16 x i8> %cast
 }
+
+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> <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>)
+;
+  %res = bitcast float 1.0 to <1 x i32>
+  ret <1 x i32> %res
+}

nikic · 2024-10-04T16:44:06Z

llvm/test/Transforms/InstSimplify/bitcast-vector-fold.ll

+; CONSTVEC-NEXT:    ret <1 x i32> <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>)


This one should still fold...

Yes, but I'm focusing on fixing the bugs related to vector Constant{Int,FP} before aligning the optimisation side of things.

I'm just concerned that we'll forget about it if it's not right in front of our eyes :)

nikic

LGTM

…ConstantInt/FP. (#116787) This fixes the code quality issue reported in #111149.

[LLVM][ConstFolds] Verify a scalar src before attempting scalar->vect…

7b84164

…or bitcast transformation. It was previously safe to assume isa<Constant{Int,FP}> meant a scalar value. This is not true when use-constant-##-for-###-splat are enabled.

llvmbot added llvm:ir llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Oct 4, 2024

nikic reviewed Oct 4, 2024

View reviewed changes

nikic approved these changes Oct 7, 2024

View reviewed changes

paulwalker-arm merged commit 87cdc83 into llvm:main Oct 8, 2024
13 checks passed

paulwalker-arm deleted the vector-constant-ir branch October 8, 2024 12:28

paulwalker-arm mentioned this pull request Nov 19, 2024

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

Merged

paulwalker-arm added a commit that referenced this pull request Nov 21, 2024

[LLVM][IR] Refactor ConstantFold:FoldBitCast to fully support vector …

af641ff

…ConstantInt/FP. (#116787) This fixes the code quality issue reported in #111149.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LLVM][ConstFolds] Verify a scalar src before attempting scalar->vector bitcast transformation. #111149

[LLVM][ConstFolds] Verify a scalar src before attempting scalar->vector bitcast transformation. #111149

Uh oh!

paulwalker-arm commented Oct 4, 2024

Uh oh!

llvmbot commented Oct 4, 2024 •

edited

Loading

Uh oh!

nikic Oct 4, 2024

Uh oh!

paulwalker-arm Oct 4, 2024

Uh oh!

nikic Oct 7, 2024

Uh oh!

nikic left a comment

Uh oh!

Uh oh!

Uh oh!

[LLVM][ConstFolds] Verify a scalar src before attempting scalar->vector bitcast transformation. #111149

[LLVM][ConstFolds] Verify a scalar src before attempting scalar->vector bitcast transformation. #111149

Uh oh!

Conversation

paulwalker-arm commented Oct 4, 2024

Uh oh!

llvmbot commented Oct 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic Oct 4, 2024

Choose a reason for hiding this comment

Uh oh!

paulwalker-arm Oct 4, 2024

Choose a reason for hiding this comment

Uh oh!

nikic Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Oct 4, 2024 •

edited

Loading