[ARM] Fix vector ule zero lowering

[dmgreen]

Summary

The instruction icmp ule <4 x i32> %0, zeroinitializer will usually be simplified to icmp eq <4 x i32> %0, zeroinitializer. It is not guaranteed though, and the code for lowering vector compares could pick the wrong form of the instruction if this happened. I've tried to make the code more explicit about the supported conditions.

This fixes NEON being unable to select VCMPZ with HS conditions, and fixes some incorrect MVE patterns.
Fixes #58514.

[RKSimon]

You might be able to handle this more generally in SelectionDAG::FoldSetCC ?

[dmgreen]

Do you mean converting ule 0 -> eq 0? I wanted the code here to be correct without needing it.

[RKSimon]

In D136447#3874695, @dmgreen wrote:

Do you mean converting ule 0 -> eq 0? I wanted the code here to be correct without needing it.

yes - its called inside getNode() so will already have been run

[dmgreen]

I would still prefer the code here to be correct. It shouldn't need to rely on canonicalization to produce the correct result (or not crash).

[samtebbs]

LGTM

[aaronpuchert]

In D136447#3874695, @dmgreen wrote:

Do you mean converting ule 0 -> eq 0? I wanted the code here to be correct without needing it.

Normalizing wouldn't be entirely unprecedented in my understanding, after all we have e.g. a number of files llvm/lib/CodeGen/SelectionDAG/Legalize*. But this here also works for me.

llvm/lib/Target/ARM/ARMISelLowering.cpp

15231–15240

Does something need to be done here?

[dmgreen]

Normalizing wouldn't be entirely unprecedented in my understanding, after all we have e.g. a number of files llvm/lib/CodeGen/SelectionDAG/Legalize*. But this here also works for me.

We shouldn't leave parts of the code broken if we can fix them. Even if it was made reliable now, we don't know how the canonicalisation can change in the future.

llvm/lib/Target/ARM/ARMISelLowering.cpp

15231–15240

Hmm. Don't think so. What kind of thing do you mean?

[aaronpuchert]

llvm/lib/Target/ARM/ARMISelLowering.cpp

15231–15240

It seems to me that we're also creating ARMISD::VCMPZ nodes here, and it doesn't seem obvious to me that we're excluding the problematic ARMCC::CondCodes. But I'm probably missing something, just wanted to make sure you're aware of this function.

[dmgreen]

llvm/lib/Target/ARM/ARMISelLowering.cpp

15231–15240

I see, yeah. This code is only for MVE, and we check isValidMVECond. I think it's OK, and we should have the test coverage for it.

llvm/lib/Target/ARM/ARMISelLowering.cpp

if (Op1.getValueType().isFloatingPoint()) {
     }
   }
 
   if (Swap)
     std::swap(Op0, Op1);
 
   // If one of the operands is a constant vector zero, attempt to fold the
   // comparison to a specialized compare-against-zero form.
-  SDValue SingleOp;
-  if (ISD::isBuildVectorAllZeros(Op1.getNode()))
-    SingleOp = Op0;
-  else if (ISD::isBuildVectorAllZeros(Op0.getNode())) {
+  if (ISD::isBuildVectorAllZeros(Op0.getNode()) &&
+      (Opc == ARMCC::GE || Opc == ARMCC::GT || Opc == ARMCC::EQ ||
+       Opc == ARMCC::NE)) {
     if (Opc == ARMCC::GE)
       Opc = ARMCC::LE;
     else if (Opc == ARMCC::GT)
       Opc = ARMCC::LT;
-    SingleOp = Op1;
+    std::swap(Op0, Op1);
   }
 
   SDValue Result;
-  if (SingleOp.getNode()) {
-    Result = DAG.getNode(ARMISD::VCMPZ, dl, CmpVT, SingleOp,
+  if (ISD::isBuildVectorAllZeros(Op1.getNode()) &&
+      (Opc == ARMCC::GE || Opc == ARMCC::GT || Opc == ARMCC::LE ||
+       Opc == ARMCC::LT || Opc == ARMCC::NE || Opc == ARMCC::EQ))
+    Result = DAG.getNode(ARMISD::VCMPZ, dl, CmpVT, Op0,
                          DAG.getConstant(Opc, dl, MVT::i32));
-  } else {
+  else
     Result = DAG.getNode(ARMISD::VCMP, dl, CmpVT, Op0, Op1,
                          DAG.getConstant(Opc, dl, MVT::i32));
-  }
 
   Result = DAG.getSExtOrTrunc(Result, dl, VT);
 
   if (Invert)
     Result = DAG.getNOT(dl, Result, VT);
 
   return Result;
 }
static SDValue PerformVCMPCombine(SDNode *N, SelectionDAG &DAG,
 
   EVT VT = N->getValueType(0);
   SDValue Op0 = N->getOperand(0);
   SDValue Op1 = N->getOperand(1);
   ARMCC::CondCodes Cond =
       (ARMCC::CondCodes)cast<ConstantSDNode>(N->getOperand(2))->getZExtValue();
   SDLoc dl(N);
 
   // vcmp X, 0, cc -> vcmpz X, cc
   if (isZeroVector(Op1))
     return DAG.getNode(ARMISD::VCMPZ, dl, VT, Op0, N->getOperand(2));
 
   unsigned SwappedCond = getSwappedCondition(Cond);
   if (isValidMVECond(SwappedCond, VT.isFloatingPoint())) {
     // vcmp 0, X, cc -> vcmpz X, reversed(cc)
     if (isZeroVector(Op0))
       return DAG.getNode(ARMISD::VCMPZ, dl, VT, Op1,
                          DAG.getConstant(SwappedCond, dl, MVT::i32));
     // vcmp vdup(Y), X, cc -> vcmp X, vdup(Y), reversed(cc)
     if (Op0->getOpcode() == ARMISD::VDUP && Op1->getOpcode() != ARMISD::VDUP)
       return DAG.getNode(ARMISD::VCMP, dl, VT, Op1, Op0,
                          DAG.getConstant(SwappedCond, dl, MVT::i32));
   }
 
   return SDValue();
 }

llvm/test/CodeGen/ARM/vcmpz.ll

 ; CHECK:       @ %bb.0:
 ; CHECK-NEXT:    vmov.i32 q0, #0x0
 ; CHECK-NEXT:    bx lr
   %2 = icmp ult <4 x i32> %0, zeroinitializer
   %3 = sext <4 x i1> %2 to <4 x i32>
   ret <4 x i32> %3
 }
 
-;define arm_aapcs_vfpcc <4 x i32> @vcmpz_zr_ule(<4 x i32> %0) {
-;  %2 = icmp ule <4 x i32> %0, zeroinitializer
-;  %3 = sext <4 x i1> %2 to <4 x i32>
-;  ret <4 x i32> %3
-;}
+define arm_aapcs_vfpcc <4 x i32> @vcmpz_zr_ule(<4 x i32> %0) {
+; CHECK-LABEL: vcmpz_zr_ule:
+; CHECK:       @ %bb.0:
+; CHECK-NEXT:    vmov.i32 q8, #0x0
+; CHECK-NEXT:    vcge.u32 q0, q8, q0
+; CHECK-NEXT:    bx lr
+  %2 = icmp ule <4 x i32> %0, zeroinitializer
+  %3 = sext <4 x i1> %2 to <4 x i32>
+  ret <4 x i32> %3
+}
 
 define arm_aapcs_vfpcc <4 x i32> @vcmpz_zr_ugt(<4 x i32> %0) {
 ; CHECK-LABEL: vcmpz_zr_ugt:
 ; CHECK:       @ %bb.0:
 ; CHECK-NEXT:    vceq.i32 q8, q0, #0
 ; CHECK-NEXT:    vmvn q0, q8
 ; CHECK-NEXT:    bx lr
   %2 = icmp ugt <4 x i32> %0, zeroinitializer

 ; CHECK:       @ %bb.0:
 ; CHECK-NEXT:    vmov.i32 q0, #0x0
 ; CHECK-NEXT:    bx lr
   %2 = icmp ugt <4 x i32> zeroinitializer, %0
   %3 = sext <4 x i1> %2 to <4 x i32>
   ret <4 x i32> %3
 }
 
-;define arm_aapcs_vfpcc <4 x i32> @vcmpz_zl_uge(<4 x i32> %0) {
-;  %2 = icmp uge <4 x i32> zeroinitializer, %0
-;  %3 = sext <4 x i1> %2 to <4 x i32>
-;  ret <4 x i32> %3
-;}
+define arm_aapcs_vfpcc <4 x i32> @vcmpz_zl_uge(<4 x i32> %0) {
+; CHECK-LABEL: vcmpz_zl_uge:
+; CHECK:       @ %bb.0:
+; CHECK-NEXT:    vmov.i32 q8, #0x0
+; CHECK-NEXT:    vcge.u32 q0, q8, q0
+; CHECK-NEXT:    bx lr
+  %2 = icmp uge <4 x i32> zeroinitializer, %0
+  %3 = sext <4 x i1> %2 to <4 x i32>
+  ret <4 x i32> %3
+}

llvm/test/CodeGen/Thumb2/mve-pred-and.ll

entry:
   %o = and <4 x i1> %c1, %c2
   %s = select <4 x i1> %o, <4 x i32> %a, <4 x i32> %b
   ret <4 x i32> %s
 }
 
 define arm_aapcs_vfpcc <4 x i32> @cmpulez_v4i1(<4 x i32> %a, <4 x i32> %b) {
 ; CHECK-LABEL: cmpulez_v4i1:
 ; CHECK:       @ %bb.0: @ %entry
+; CHECK-NEXT:    vmov.i32 q2, #0x0
 ; CHECK-NEXT:    vpt.i32 eq, q0, zr
-; CHECK-NEXT:    vcmpt.u32 cs, q1, zr
+; CHECK-NEXT:    vcmpt.u32 cs, q2, q1
 ; CHECK-NEXT:    vpsel q0, q0, q1
 ; CHECK-NEXT:    bx lr
 entry:
   %c1 = icmp eq <4 x i32> %a, zeroinitializer
   %c2 = icmp ule <4 x i32> %b, zeroinitializer
   %o = and <4 x i1> %c1, %c2
   %s = select <4 x i1> %o, <4 x i32> %a, <4 x i32> %b
   ret <4 x i32> %s

llvm/test/CodeGen/Thumb2/mve-pred-or.ll

entry:
   %o = or <4 x i1> %c1, %c2
   %s = select <4 x i1> %o, <4 x i32> %a, <4 x i32> %b
   ret <4 x i32> %s
 }
 
 define arm_aapcs_vfpcc <4 x i32> @cmpulez_v4i1(<4 x i32> %a, <4 x i32> %b) {
 ; CHECK-LABEL: cmpulez_v4i1:
 ; CHECK:       @ %bb.0: @ %entry
-; CHECK-NEXT:    vcmp.u32 cs, q1, zr
+; CHECK-NEXT:    vmov.i32 q2, #0x0
+; CHECK-NEXT:    vcmp.u32 cs, q2, q1
 ; CHECK-NEXT:    vpnot
 ; CHECK-NEXT:    vpst
 ; CHECK-NEXT:    vcmpt.i32 ne, q0, zr
 ; CHECK-NEXT:    vpsel q0, q1, q0
 ; CHECK-NEXT:    bx lr
 entry:
   %c1 = icmp eq <4 x i32> %a, zeroinitializer
   %c2 = icmp ule <4 x i32> %b, zeroinitializer

llvm/test/CodeGen/Thumb2/mve-pred-xor.ll

entry:
   %o = xor <4 x i1> %c1, %c2
   %s = select <4 x i1> %o, <4 x i32> %a, <4 x i32> %b
   ret <4 x i32> %s
 }
 
 define arm_aapcs_vfpcc <4 x i32> @cmpulez_v4i1(<4 x i32> %a, <4 x i32> %b) {
 ; CHECK-LABEL: cmpulez_v4i1:
 ; CHECK:       @ %bb.0: @ %entry
-; CHECK-NEXT:    vcmp.u32 cs, q1, zr
+; CHECK-NEXT:    vmov.i32 q2, #0x0
+; CHECK-NEXT:    vcmp.u32 cs, q2, q1
 ; CHECK-NEXT:    vmrs r0, p0
 ; CHECK-NEXT:    vcmp.i32 eq, q0, zr
 ; CHECK-NEXT:    vmrs r1, p0
 ; CHECK-NEXT:    eors r0, r1
 ; CHECK-NEXT:    vmsr p0, r0
 ; CHECK-NEXT:    vpsel q0, q0, q1
 ; CHECK-NEXT:    bx lr
 entry:

llvm/test/CodeGen/Thumb2/mve-vcmpz.ll

entry:
   %c = icmp ult <4 x i32> %src, zeroinitializer
   %s = select <4 x i1> %c, <4 x i32> %a, <4 x i32> %b
   ret <4 x i32> %s
 }
 
 define arm_aapcs_vfpcc <4 x i32> @vcmp_ulez_v4i32(<4 x i32> %src, <4 x i32> %a, <4 x i32> %b) {
 ; CHECK-LABEL: vcmp_ulez_v4i32:
 ; CHECK:       @ %bb.0: @ %entry
-; CHECK-NEXT:    vcmp.u32 cs, q0, zr
+; CHECK-NEXT:    vmov.i32 q3, #0x0
+; CHECK-NEXT:    vcmp.u32 cs, q3, q0
 ; CHECK-NEXT:    vpsel q0, q1, q2
 ; CHECK-NEXT:    bx lr
 entry:
   %c = icmp ule <4 x i32> %src, zeroinitializer
   %s = select <4 x i1> %c, <4 x i32> %a, <4 x i32> %b
   ret <4 x i32> %s
 }
 
entry:
   %c = icmp ult <8 x i16> %src, zeroinitializer
   %s = select <8 x i1> %c, <8 x i16> %a, <8 x i16> %b
   ret <8 x i16> %s
 }
 
 define arm_aapcs_vfpcc <8 x i16> @vcmp_ulez_v8i16(<8 x i16> %src, <8 x i16> %a, <8 x i16> %b) {
 ; CHECK-LABEL: vcmp_ulez_v8i16:
 ; CHECK:       @ %bb.0: @ %entry
-; CHECK-NEXT:    vcmp.u16 cs, q0, zr
+; CHECK-NEXT:    vmov.i32 q3, #0x0
+; CHECK-NEXT:    vcmp.u16 cs, q3, q0
 ; CHECK-NEXT:    vpsel q0, q1, q2
 ; CHECK-NEXT:    bx lr
 entry:
   %c = icmp ule <8 x i16> %src, zeroinitializer
   %s = select <8 x i1> %c, <8 x i16> %a, <8 x i16> %b
   ret <8 x i16> %s
 }
 
entry:
   %c = icmp ult <16 x i8> %src, zeroinitializer
   %s = select <16 x i1> %c, <16 x i8> %a, <16 x i8> %b
   ret <16 x i8> %s
 }
 
 define arm_aapcs_vfpcc <16 x i8> @vcmp_ulez_v16i8(<16 x i8> %src, <16 x i8> %a, <16 x i8> %b) {
 ; CHECK-LABEL: vcmp_ulez_v16i8:
 ; CHECK:       @ %bb.0: @ %entry
-; CHECK-NEXT:    vcmp.u8 cs, q0, zr
+; CHECK-NEXT:    vmov.i32 q3, #0x0
+; CHECK-NEXT:    vcmp.u8 cs, q3, q0
 ; CHECK-NEXT:    vpsel q0, q1, q2
 ; CHECK-NEXT:    bx lr
 entry:
   %c = icmp ule <16 x i8> %src, zeroinitializer
   %s = select <16 x i1> %c, <16 x i8> %a, <16 x i8> %b
   ret <16 x i8> %s
 }
 
entry:
   %c = icmp ugt <4 x i32> zeroinitializer, %src
   %s = select <4 x i1> %c, <4 x i32> %a, <4 x i32> %b
   ret <4 x i32> %s
 }
 
 define arm_aapcs_vfpcc <4 x i32> @vcmp_r_ugez_v4i32(<4 x i32> %src, <4 x i32> %a, <4 x i32> %b) {
 ; CHECK-LABEL: vcmp_r_ugez_v4i32:
 ; CHECK:       @ %bb.0: @ %entry
-; CHECK-NEXT:    vcmp.u32 cs, q0, zr
+; CHECK-NEXT:    vmov.i32 q3, #0x0
+; CHECK-NEXT:    vcmp.u32 cs, q3, q0
 ; CHECK-NEXT:    vpsel q0, q1, q2
 ; CHECK-NEXT:    bx lr
 entry:
   %c = icmp uge <4 x i32> zeroinitializer, %src
   %s = select <4 x i1> %c, <4 x i32> %a, <4 x i32> %b
   ret <4 x i32> %s
 }
 
entry:
   %c = icmp ugt <8 x i16> zeroinitializer, %src
   %s = select <8 x i1> %c, <8 x i16> %a, <8 x i16> %b
   ret <8 x i16> %s
 }
 
 define arm_aapcs_vfpcc <8 x i16> @vcmp_r_ugez_v8i16(<8 x i16> %src, <8 x i16> %a, <8 x i16> %b) {
 ; CHECK-LABEL: vcmp_r_ugez_v8i16:
 ; CHECK:       @ %bb.0: @ %entry
-; CHECK-NEXT:    vcmp.u16 cs, q0, zr
+; CHECK-NEXT:    vmov.i32 q3, #0x0
+; CHECK-NEXT:    vcmp.u16 cs, q3, q0
 ; CHECK-NEXT:    vpsel q0, q1, q2
 ; CHECK-NEXT:    bx lr
 entry:
   %c = icmp uge <8 x i16> zeroinitializer, %src
   %s = select <8 x i1> %c, <8 x i16> %a, <8 x i16> %b
   ret <8 x i16> %s
 }
 
entry:
   %c = icmp ugt <16 x i8> zeroinitializer, %src
   %s = select <16 x i1> %c, <16 x i8> %a, <16 x i8> %b
   ret <16 x i8> %s
 }
 
 define arm_aapcs_vfpcc <16 x i8> @vcmp_r_ugez_v16i8(<16 x i8> %src, <16 x i8> %a, <16 x i8> %b) {
 ; CHECK-LABEL: vcmp_r_ugez_v16i8:
 ; CHECK:       @ %bb.0: @ %entry
-; CHECK-NEXT:    vcmp.u8 cs, q0, zr
+; CHECK-NEXT:    vmov.i32 q3, #0x0
+; CHECK-NEXT:    vcmp.u8 cs, q3, q0
 ; CHECK-NEXT:    vpsel q0, q1, q2
 ; CHECK-NEXT:    bx lr
 entry:
   %c = icmp uge <16 x i8> zeroinitializer, %src
   %s = select <16 x i1> %c, <16 x i8> %a, <16 x i8> %b
   ret <16 x i8> %s
 }