[LegalizeTypes] Allow v1i128 as a valid SETCC result type during vector operand scalarization

[maryammo]

PowerPC registers v1i128 as a legal type when P8Altivec is available. When lowering <4 x fp128> comparisons, the type legalizer hits a v1i1 only assert. Generalize the assert to accept any single-element vector result type.

[llvmorg-github-actions]

@llvm/pr-subscribers-backend-powerpc

@llvm/pr-subscribers-llvm-selectiondag

Author: Maryam Moghadas (maryammo)

Changes

PowerPC registers v1i128 as a legal type when P8Altivec is available. When lowering <4 x fp128> comparisons, the type legalizer hits a v1i1 only assert. Relax the assert to also accept v1i128.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp (+9-4)
• (added) llvm/test/CodeGen/PowerPC/fp128-vector-setcc.ll (+22)

diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
index 19b221bb98d46..a3f4f5c573e23 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
@@ -1114,13 +1114,16 @@ SDValue DAGTypeLegalizer::ScalarizeVecOp_VSELECT(SDNode *N) {
 }
 
 /// If the operand is a vector that needs to be scalarized then the
-/// result must be v1i1, so just convert to a scalar SETCC and wrap
-/// with a scalar_to_vector since the res type is legal if we got here
+/// result must be v1i1 or v1i128, so just convert to a scalar SETCC
+/// and wrap with a scalar_to_vector since the res type is legal if we
+/// got here
 SDValue DAGTypeLegalizer::ScalarizeVecOp_VSETCC(SDNode *N) {
   assert(N->getValueType(0).isVector() &&
          N->getOperand(0).getValueType().isVector() &&
          "Operand types must be vectors");
-  assert(N->getValueType(0) == MVT::v1i1 && "Expected v1i1 type");
+  assert(
+      (N->getValueType(0) == MVT::v1i1 || N->getValueType(0) == MVT::v1i128) &&
+      "Expected v1i1 or v1i128 type");
 
   EVT VT = N->getValueType(0);
   SDValue LHS = GetScalarizedVector(N->getOperand(0));
@@ -1150,7 +1153,9 @@ SDValue DAGTypeLegalizer::ScalarizeVecOp_VSTRICT_FSETCC(SDNode *N,
   assert(N->getValueType(0).isVector() &&
          N->getOperand(1).getValueType().isVector() &&
          "Operand types must be vectors");
-  assert(N->getValueType(0) == MVT::v1i1 && "Expected v1i1 type");
+  assert(
+      (N->getValueType(0) == MVT::v1i1 || N->getValueType(0) == MVT::v1i128) &&
+      "Expected v1i1 or v1i128 type");
 
   EVT VT = N->getValueType(0);
   SDValue Ch = N->getOperand(0);
diff --git a/llvm/test/CodeGen/PowerPC/fp128-vector-setcc.ll b/llvm/test/CodeGen/PowerPC/fp128-vector-setcc.ll
new file mode 100644
index 0000000000000..1d0c83edd8b78
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/fp128-vector-setcc.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 %s -o - | FileCheck %s
+
+define <4 x i1> @fp(<4 x fp128> %0)  {
+; CHECK-LABEL: fp:
+; CHECK-COUNT-4: bl __eqkf2
+; CHECK: blr
+Entry:
+  %1 = fcmp oeq <4 x fp128> %0, zeroinitializer
+  ret <4 x i1> %1
+}
+
+define <4 x i1> @foo(<4 x fp128> %0) strictfp {
+; CHECK-LABEL: foo:
+; CHECK-COUNT-4: bl __eqkf2
+; CHECK: blr
+Entry:
+  %1 = call <4 x i1> @llvm.experimental.constrained.fcmp.v4fp128(<4 x fp128> %0, <4 x fp128> zeroinitializer, metadata !"oeq", metadata !"fpexcept.strict")
+  ret <4 x i1> %1
+}
+
+declare <4 x i1> @llvm.experimental.constrained.fcmp.v4fp128(<4 x fp128>, <4 x fp128>, metadata, metadata)
+

[arsenm]

I don't see the point of this assert

[maryammo]

Updated to getVectorNumElements() == 1 as suggested below.

[RKSimon]

do we even need the assert? or could it be relaxed to:
N->getValueType(0).getVectorMinNumElements() == 1?

[topperc]

do we even need the assert? or could it be relaxed to: N->getValueType(0).getVectorMinNumElements() == 1?

Why getVectorMinNumElements() instead of getVectornNumElements()?

[RKSimon]

do we even need the assert? or could it be relaxed to: N->getValueType(0).getVectorMinNumElements() == 1?

Why getVectorMinNumElements() instead of getVectornNumElements()?

getVectorNumElements() is even better :)

[topperc]

LGTM

[RKSimon]

LGTM

[alexrp]

Good to land? Would be nice to get it into 23.x before the release.