[SVE] Fix SVEDup0 matching -0.0f

[steplong]

Summary

Because of D128669, CPY is being used to zero active lanes even in the case of -0.0f. This patch checks for floating point positive zero. That way SVEDup0 won't match -0.0f.

Fixes https://github.com/llvm/llvm-project/issues/57428

[steplong]

I'm not sure how to test this. Also how do I reference the issue I created on Github: https://github.com/llvm/llvm-project/issues/57428?

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

185–186

Does it make sense to check for isNegative here also?

[efriedma]

Change seems reasonable.

Does https://github.com/llvm/llvm-project/blob/04ae35e592c1e7e99bb3894420b6ff2117ace78a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp#L2698 have the same issue?

We need regression tests.

[efriedma]

Also how do I reference the issue I created on Github

To refer to the issue, just put "Fixes https://github.com/llvm/llvm-project/issues/57428" in the commit message. GitHub will cross-link that automatically.

I'm not sure how to test this.

Backend regression tests generally go into llvm/test/CodeGen/AArch64. You can probably just add a few lines to the existing test llvm/test/CodeGen/AArch64/sve-vselect-imm.ll to cover this case.

[paulwalker-arm]

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

185–186

Yes please.

203–204

It looks like we can wrap these into a single isNullFPConstant() call?

[steplong]

• Add tests for SVEDup0 case
• Change to isNullFPConstant and isNullConstant

[Allen]

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

187

sorry, I have a question: can we match the -0.0 if the option -ffast-math is added in command ?

[paulwalker-arm]

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

187

That feels a little dangerous to me. I think such a canonicalisation would be better done at the DAG or perhaps even the IR level. For instruction selection it seems safer to be explicit.

[Allen]

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

187

Thanks for your information.

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

public:
   }
 
   bool SelectDupZeroOrUndef(SDValue N) {
     switch(N->getOpcode()) {
     case ISD::UNDEF:
       return true;
     case AArch64ISD::DUP:
     case ISD::SPLAT_VECTOR: {
       auto Opnd0 = N->getOperand(0);
-      if (auto CN = dyn_cast<ConstantSDNode>(Opnd0))
-        if (CN->isZero())
-          return true;
-      if (auto CN = dyn_cast<ConstantFPSDNode>(Opnd0))
-        if (CN->isZero())
-          return true;
+      if (isNullConstant(Opnd0))
+      if (isNullFPConstant(Opnd0))
       break;
     }
     default:
       break;
     }
 
     return false;
   }
 
   bool SelectDupZero(SDValue N) {
     switch(N->getOpcode()) {
     case AArch64ISD::DUP:
     case ISD::SPLAT_VECTOR: {
       auto Opnd0 = N->getOperand(0);
-      if (auto CN = dyn_cast<ConstantSDNode>(Opnd0))
-        if (CN->isZero())
-          return true;
-      if (auto CN = dyn_cast<ConstantFPSDNode>(Opnd0))
-        if (CN->isZero())
-          return true;
+      if (isNullConstant(Opnd0))
+      if (isNullFPConstant(Opnd0))
       break;
     }
     }
 
     return false;
   }
 
   template<MVT::SimpleValueType VT>

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

static bool isZerosVector(const SDNode *N) {
 
   if (ISD::isConstantSplatVectorAllZeros(N))
     return true;
 
   if (N->getOpcode() != AArch64ISD::DUP)
     return false;
 
   auto Opnd0 = N->getOperand(0);
-  auto *CINT = dyn_cast<ConstantSDNode>(Opnd0);
-  auto *CFP = dyn_cast<ConstantFPSDNode>(Opnd0);
-  return (CINT && CINT->isZero()) || (CFP && CFP->isZero());
+  return isNullConstant(Opnd0) || isNullFPConstant(Opnd0);
 }
 
 /// changeIntCCToAArch64CC - Convert a DAG integer condition code to an AArch64
 /// CC
 static AArch64CC::CondCode changeIntCCToAArch64CC(ISD::CondCode CC) {
   switch (CC) {
   default:
     llvm_unreachable("Unknown condition code!");

llvm/test/CodeGen/AArch64/sve-vselect-imm.ll

 ; CHECK-LABEL: sel_merge_nx2f64_zero:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    mov z0.d, p0/m, #0 // =0x0
 ; CHECK-NEXT:    ret
 %sel = select <vscale x 2 x i1> %p, <vscale x 2 x double> zeroinitializer, <vscale x 2 x double> %in
 ret <vscale x 2 x double> %sel
 }
 
+define <vscale x 8 x half> @sel_merge_nxv8f16_negative_zero(<vscale x 8 x i1> %p, <vscale x 8 x half> %in) {
+; CHECK-LABEL: sel_merge_nxv8f16_negative_zero:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov w8, #32768
+; CHECK-NEXT:    mov z1.h, w8
+; CHECK-NEXT:    mov z0.h, p0/m, z1.h
+; CHECK-NEXT:    ret
+%vec = shufflevector <vscale x 8 x half> insertelement (<vscale x 8 x half> undef, half -0.0, i32 0), <vscale x 8 x half> zeroinitializer, <vscale x 8 x i32> zeroinitializer
+%sel = select <vscale x 8 x i1> %p, <vscale x 8 x half> %vec, <vscale x 8 x half> %in
+ret <vscale x 8 x half> %sel
+}
+
+define <vscale x 4 x half> @sel_merge_nx4f16_negative_zero(<vscale x 4 x i1> %p, <vscale x 4 x half> %in) {
+; CHECK-LABEL: sel_merge_nx4f16_negative_zero:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov w8, #32768
+; CHECK-NEXT:    mov z1.h, w8
+; CHECK-NEXT:    mov z0.s, p0/m, z1.s
+; CHECK-NEXT:    ret
+%vec = shufflevector <vscale x 4 x half> insertelement (<vscale x 4 x half> undef, half -0.0, i32 0), <vscale x 4 x half> zeroinitializer, <vscale x 4 x i32> zeroinitializer
+%sel = select <vscale x 4 x i1> %p, <vscale x 4 x half> %vec, <vscale x 4 x half> %in
+ret <vscale x 4 x half> %sel
+}
+
+define <vscale x 2 x half> @sel_merge_nx2f16_negative_zero(<vscale x 2 x i1> %p, <vscale x 2 x half> %in) {
+; CHECK-LABEL: sel_merge_nx2f16_negative_zero:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov w8, #32768
+; CHECK-NEXT:    mov z1.h, w8
+; CHECK-NEXT:    mov z0.d, p0/m, z1.d
+; CHECK-NEXT:    ret
+%vec = shufflevector <vscale x 2 x half> insertelement (<vscale x 2 x half> undef, half -0.0, i32 0), <vscale x 2 x half> zeroinitializer, <vscale x 2 x i32> zeroinitializer
+%sel = select <vscale x 2 x i1> %p, <vscale x 2 x half> %vec, <vscale x 2 x half> %in
+ret <vscale x 2 x half> %sel
+}
+
+define <vscale x 4 x float> @sel_merge_nx4f32_negative_zero(<vscale x 4 x i1> %p, <vscale x 4 x float> %in) {
+; CHECK-LABEL: sel_merge_nx4f32_negative_zero:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov w8, #-2147483648
+; CHECK-NEXT:    mov z1.s, w8
+; CHECK-NEXT:    mov z0.s, p0/m, z1.s
+; CHECK-NEXT:    ret
+%vec = shufflevector <vscale x 4 x float> insertelement (<vscale x 4 x float> undef, float -0.0, i32 0), <vscale x 4 x float> zeroinitializer, <vscale x 4 x i32> zeroinitializer
+%sel = select <vscale x 4 x i1> %p, <vscale x 4 x float> %vec, <vscale x 4 x float> %in
+ret <vscale x 4 x float> %sel
+}
+
+define <vscale x 2 x float> @sel_merge_nx2f32_negative_zero(<vscale x 2 x i1> %p, <vscale x 2 x float> %in) {
+; CHECK-LABEL: sel_merge_nx2f32_negative_zero:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov w8, #-2147483648
+; CHECK-NEXT:    mov z1.s, w8
+; CHECK-NEXT:    mov z0.d, p0/m, z1.d
+; CHECK-NEXT:    ret
+%vec = shufflevector <vscale x 2 x float> insertelement (<vscale x 2 x float> undef, float -0.0, i32 0), <vscale x 2 x float> zeroinitializer, <vscale x 2 x i32> zeroinitializer
+%sel = select <vscale x 2 x i1> %p, <vscale x 2 x float> %vec, <vscale x 2 x float> %in
+ret <vscale x 2 x float> %sel
+}
+
+define <vscale x 2 x double> @sel_merge_nx2f64_negative_zero(<vscale x 2 x i1> %p, <vscale x 2 x double> %in) {
+; CHECK-LABEL: sel_merge_nx2f64_negative_zero:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov x8, #-9223372036854775808
+; CHECK-NEXT:    mov z1.d, x8
+; CHECK-NEXT:    mov z0.d, p0/m, z1.d
+; CHECK-NEXT:    ret
+%vec = shufflevector <vscale x 2 x double> insertelement (<vscale x 2 x double> undef, double -0.0, i32 0), <vscale x 2 x double> zeroinitializer, <vscale x 2 x i32> zeroinitializer
+%sel = select <vscale x 2 x i1> %p, <vscale x 2 x double> %vec, <vscale x 2 x double> %in
+ret <vscale x 2 x double> %sel
+}
+
 define <vscale x 8 x i16> @sel_merge_16_shifted(<vscale x 8 x i1> %p, <vscale x 8 x i16> %in) {
 ; CHECK-LABEL: sel_merge_16_shifted:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    mov z0.h, p0/m, #512 // =0x200
 ; CHECK-NEXT:    ret
   %vec = shufflevector <vscale x 8 x i16> insertelement (<vscale x 8 x i16> undef, i16 512, i32 0), <vscale x 8 x i16> zeroinitializer, <vscale x 8 x i32> zeroinitializer
   %sel = select <vscale x 8 x i1> %p, <vscale x 8 x i16> %vec, <vscale x 8 x i16> %in
   ret <vscale x 8 x i16> %sel