[llvm] [InstCombine] Avoid crash on aggregate types in SimplifyDemandedUseFPClass (PR #111128)

Benjamin Maxwell via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 4 05:41:30 PDT 2024


https://github.com/MacDue updated https://github.com/llvm/llvm-project/pull/111128

>From d92961a7c80807b3e706a9ef552a68ff11f3e41a Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Fri, 4 Oct 2024 10:14:37 +0000
Subject: [PATCH 1/4] [InstCombine] Avoid crash on aggregate types in
 SimplifyDemandedUseFPClass

The disables folding to FP aggregates that are not poison types, which
is currently not supported. Note: To fully handle this case would also
likely require teaching `computeKnownFPClass()` to handle array and
struct constants (which does not seem implemented outside of zero init).
---
 .../InstCombineSimplifyDemanded.cpp           | 13 +++++++++
 .../InstCombine/simplify-demanded-fpclass.ll  | 29 +++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index ee6b60f7f70d68..cb8c0dff7ba7dd 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -1919,6 +1919,19 @@ Value *InstCombinerImpl::SimplifyDemandedVectorElts(Value *V,
 /// For floating-point classes that resolve to a single bit pattern, return that
 /// value.
 static Constant *getFPClassConstant(Type *Ty, FPClassTest Mask) {
+  // TODO: Support aggregate types that are allowed by FPMathOperator.
+  switch (Mask) {
+    case fcPosZero:
+    case fcNegZero:
+    case fcPosInf:
+    case fcNegInf:
+      if (Ty->isAggregateType())
+        return nullptr;
+      break;
+    default:
+      break;
+  }
+
   switch (Mask) {
   case fcPosZero:
     return ConstantFP::getZero(Ty);
diff --git a/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll b/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll
index 403f3bacf34d89..c03c31f4f41ba7 100644
--- a/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll
+++ b/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll
@@ -91,6 +91,35 @@ define nofpclass(inf) float @ret_nofpclass_inf__ninf() {
   ret float 0xFFF0000000000000
 }
 
+; Basic aggregate tests to ensure this does not crash.
+define nofpclass(nan) { float } @ret_nofpclass_struct_ty() {
+; CHECK-LABEL: define nofpclass(nan) { float } @ret_nofpclass_struct_ty() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret { float } zeroinitializer
+;
+entry:
+  ret { float } zeroinitializer
+}
+
+
+define nofpclass(nan) [ 5 x float ] @ret_nofpclass_array_ty() {
+; CHECK-LABEL: define nofpclass(nan) [5 x float] @ret_nofpclass_array_ty() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret [5 x float] zeroinitializer
+;
+entry:
+  ret [ 5 x float ] zeroinitializer
+}
+
+define nofpclass(nan) [ 2 x [ 5 x float ]] @ret_nofpclass_nested_array_ty() {
+; CHECK-LABEL: define nofpclass(nan) [2 x [5 x float]] @ret_nofpclass_nested_array_ty() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret [2 x [5 x float]] zeroinitializer
+;
+entry:
+  ret [ 2 x [ 5 x float ]] zeroinitializer
+}
+
 ; Negative test, do nothing
 define nofpclass(inf) float @ret_nofpclass_inf__select_nofpclass_inf_lhs(i1 %cond, float nofpclass(inf) %x, float %y) {
 ; CHECK-LABEL: define nofpclass(inf) float @ret_nofpclass_inf__select_nofpclass_inf_lhs

>From d8f6a6b3c001139dd6df14f306f22b2239bdb70b Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Fri, 4 Oct 2024 10:44:33 +0000
Subject: [PATCH 2/4] Fixups

---
 .../InstCombineSimplifyDemanded.cpp           | 26 +++---
 .../InstCombine/simplify-demanded-fpclass.ll  | 92 +++++++++++++++++++
 2 files changed, 103 insertions(+), 15 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index cb8c0dff7ba7dd..1ffa8281c4d9df 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -1919,30 +1919,26 @@ Value *InstCombinerImpl::SimplifyDemandedVectorElts(Value *V,
 /// For floating-point classes that resolve to a single bit pattern, return that
 /// value.
 static Constant *getFPClassConstant(Type *Ty, FPClassTest Mask) {
-  // TODO: Support aggregate types that are allowed by FPMathOperator.
-  switch (Mask) {
-    case fcPosZero:
-    case fcNegZero:
-    case fcPosInf:
-    case fcNegInf:
-      if (Ty->isAggregateType())
-        return nullptr;
-      break;
-    default:
-      break;
+  if (Mask == fcNone)
+    return PoisonValue::get(Ty);
+
+  if (Mask == fcPosZero) {
+    if (Ty->isAggregateType())
+      return ConstantAggregateZero::get(Ty);
+    return ConstantFP::getZero(Ty);
   }
 
+  // TODO: Support aggregate types that are allowed by FPMathOperator.
+  if (Ty->isAggregateType())
+    return nullptr;
+
   switch (Mask) {
-  case fcPosZero:
-    return ConstantFP::getZero(Ty);
   case fcNegZero:
     return ConstantFP::getZero(Ty, true);
   case fcPosInf:
     return ConstantFP::getInfinity(Ty);
   case fcNegInf:
     return ConstantFP::getInfinity(Ty, true);
-  case fcNone:
-    return PoisonValue::get(Ty);
   default:
     return nullptr;
   }
diff --git a/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll b/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll
index c03c31f4f41ba7..8d2a092e0f48f4 100644
--- a/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll
+++ b/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll
@@ -101,6 +101,23 @@ entry:
   ret { float } zeroinitializer
 }
 
+define nofpclass(nan) { float, float } @ret_nofpclass_multiple_elems_struct_ty() {
+; CHECK-LABEL: define nofpclass(nan) { float, float } @ret_nofpclass_multiple_elems_struct_ty() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret { float, float } zeroinitializer
+;
+entry:
+  ret { float, float } zeroinitializer
+}
+
+define nofpclass(nan) { <4 x float>, <4 x float> } @ret_nofpclass_vector_elems_struct_ty() {
+; CHECK-LABEL: define nofpclass(nan) { <4 x float>, <4 x float> } @ret_nofpclass_vector_elems_struct_ty() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret { <4 x float>, <4 x float> } zeroinitializer
+;
+entry:
+  ret { <4 x float>, <4 x float> } zeroinitializer
+}
 
 define nofpclass(nan) [ 5 x float ] @ret_nofpclass_array_ty() {
 ; CHECK-LABEL: define nofpclass(nan) [5 x float] @ret_nofpclass_array_ty() {
@@ -120,6 +137,81 @@ entry:
   ret [ 2 x [ 5 x float ]] zeroinitializer
 }
 
+define nofpclass(pinf) { float } @ret_nofpclass_struct_ty_pinf__ninf() {
+; CHECK-LABEL: define nofpclass(pinf) { float } @ret_nofpclass_struct_ty_pinf__ninf() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret { float } { float 0xFFF0000000000000 }
+;
+entry:
+  ret { float } { float 0xFFF0000000000000 }
+}
+
+define nofpclass(pinf) { float, float } @ret_nofpclass_multiple_elems_struct_ty_pinf__ninf() {
+; CHECK-LABEL: define nofpclass(pinf) { float, float } @ret_nofpclass_multiple_elems_struct_ty_pinf__ninf() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret { float, float } { float 0xFFF0000000000000, float 0xFFF0000000000000 }
+;
+entry:
+  ret { float, float } { float 0xFFF0000000000000, float 0xFFF0000000000000 }
+}
+
+define nofpclass(pinf) { <2 x float> } @ret_nofpclass_vector_elems_struct_ty_pinf__ninf() {
+; CHECK-LABEL: define nofpclass(pinf) { <2 x float> } @ret_nofpclass_vector_elems_struct_ty_pinf__ninf() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret { <2 x float> } { <2 x float> <float 0xFFF0000000000000, float 0xFFF0000000000000> }
+;
+entry:
+  ret { <2 x float>} { <2 x float> <float 0xFFF0000000000000, float 0xFFF0000000000000> }
+}
+
+
+define nofpclass(pinf) [ 1 x [ 1 x float ]] @ret_nofpclass_nested_array_ty_pinf__ninf() {
+; CHECK-LABEL: define nofpclass(pinf) [1 x [1 x float]] @ret_nofpclass_nested_array_ty_pinf__ninf() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret [1 x [1 x float]] [[1 x float] [float 0xFFF0000000000000]]
+;
+entry:
+  ret [ 1 x [ 1 x float ]] [[ 1 x float ] [float 0xFFF0000000000000]]
+}
+
+define nofpclass(pzero) { float, float } @ret_nofpclass_multiple_elems_struct_ty_pzero__nzero() {
+; CHECK-LABEL: define nofpclass(pzero) { float, float } @ret_nofpclass_multiple_elems_struct_ty_pzero__nzero() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret { float, float } { float -0.000000e+00, float -0.000000e+00 }
+;
+entry:
+  ret { float, float } { float -0.0, float -0.0 }
+}
+
+define nofpclass(ninf) { float, float } @ret_nofpclass_multiple_elems_struct_ty_ninf__npinf() {
+; CHECK-LABEL: define nofpclass(ninf) { float, float } @ret_nofpclass_multiple_elems_struct_ty_ninf__npinf() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret { float, float } { float 0x7FF0000000000000, float 0x7FF0000000000000 }
+;
+entry:
+  ret { float, float } { float 0x7FF0000000000000, float 0x7FF0000000000000 }
+}
+
+; FIXME (should be poison): Support computeKnownFPClass() for non-zero aggregates.
+define nofpclass(inf) { float, float } @ret_nofpclass_multiple_elems_struct_ty_inf__npinf() {
+; CHECK-LABEL: define nofpclass(inf) { float, float } @ret_nofpclass_multiple_elems_struct_ty_inf__npinf() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret { float, float } { float 0x7FF0000000000000, float 0x7FF0000000000000 }
+;
+entry:
+  ret { float, float } { float 0x7FF0000000000000, float 0x7FF0000000000000 }
+}
+
+; FIXME (should be poison): Support computeKnownFPClass() for non-zero aggregates.
+define nofpclass(nzero) [ 1 x float ] @ret_nofpclass_multiple_elems_struct_ty_nzero_nzero() {
+; CHECK-LABEL: define nofpclass(nzero) [1 x float] @ret_nofpclass_multiple_elems_struct_ty_nzero_nzero() {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret [1 x float] [float -0.000000e+00]
+;
+entry:
+  ret [ 1 x float ] [ float -0.0 ]
+}
+
 ; Negative test, do nothing
 define nofpclass(inf) float @ret_nofpclass_inf__select_nofpclass_inf_lhs(i1 %cond, float nofpclass(inf) %x, float %y) {
 ; CHECK-LABEL: define nofpclass(inf) float @ret_nofpclass_inf__select_nofpclass_inf_lhs

>From a991168929d79f04cb746ff794fea81b0f853060 Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Fri, 4 Oct 2024 11:47:38 +0000
Subject: [PATCH 3/4] Fix incorrectly generated test

---
 .../test/Transforms/InstCombine/simplify-demanded-fpclass.ll | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll b/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll
index 8d2a092e0f48f4..f1f1708333901a 100644
--- a/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll
+++ b/llvm/test/Transforms/InstCombine/simplify-demanded-fpclass.ll
@@ -164,11 +164,10 @@ entry:
   ret { <2 x float>} { <2 x float> <float 0xFFF0000000000000, float 0xFFF0000000000000> }
 }
 
-
 define nofpclass(pinf) [ 1 x [ 1 x float ]] @ret_nofpclass_nested_array_ty_pinf__ninf() {
-; CHECK-LABEL: define nofpclass(pinf) [1 x [1 x float]] @ret_nofpclass_nested_array_ty_pinf__ninf() {
+; CHECK-LABEL: @ret_nofpclass_nested_array_ty_pinf__ninf() {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    ret [1 x [1 x float]] [[1 x float] [float 0xFFF0000000000000]]
+; CHECK-NEXT:    ret {{.*}}float 0xFFF0000000000000
 ;
 entry:
   ret [ 1 x [ 1 x float ]] [[ 1 x float ] [float 0xFFF0000000000000]]

>From b7f52eb5ebc2c23e3c53fae291254cc4c5efa64a Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell <benjamin.maxwell at arm.com>
Date: Fri, 4 Oct 2024 12:40:34 +0000
Subject: [PATCH 4/4] Fixup

---
 .../Transforms/InstCombine/InstCombineSimplifyDemanded.cpp | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index 1ffa8281c4d9df..3d4461dc1a87f6 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -1922,11 +1922,8 @@ static Constant *getFPClassConstant(Type *Ty, FPClassTest Mask) {
   if (Mask == fcNone)
     return PoisonValue::get(Ty);
 
-  if (Mask == fcPosZero) {
-    if (Ty->isAggregateType())
-      return ConstantAggregateZero::get(Ty);
-    return ConstantFP::getZero(Ty);
-  }
+  if (Mask == fcPosZero)
+    return Constant::getNullValue(Ty);
 
   // TODO: Support aggregate types that are allowed by FPMathOperator.
   if (Ty->isAggregateType())



More information about the llvm-commits mailing list