[AArch64] Fix incorrect getSetCCInverse usage within trySwapVSelectOperands.

[paulwalker-arm]

Summary

When inverting the compare predicate trySwapVSelectOperands is
incorrectly using the type of the select's cond operand rather
than the type of cond's operands. This means we're treating all
inversions as if they're integer.

[david-arm]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

17522

Hi @paulwalker-arm, I actually think this is still wrong because I'm pretty sure we should be using the SetCC's input operand type, i.e.

ISD::getSetCCInverse(CC, SetCC.getOperand(0).getValueType())

The reason is that the setcc may actually use integer operands, i.e.

%p = icmp eq <vscale x 4 x i32> %c, zeroinitializer
%fmul = fmul <vscale x 4 x float> %a, %b
%sel = select <vscale x 4 x i1> %p, <vscale x 4 x float> %a, <vscale x 4 x float> %fmul

In fact, this is not your fault, but I think we're missing some tests for this. I don't see anything in trySwapVSelectOperands that precludes this possibility.

[paulwalker-arm]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

17522

Are yes, not sure what I was thinking. Thanks Dave.

[paulwalker-arm]

Fixed the bug Dave spoted and changed one of the tests to use an integer comparison.

[david-arm]

LGTM! Thanks for making the changes @paulwalker-arm. :)

[paulwalker-arm]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

17522

Although I've changed one of the tests to use an integer comparison it seems the code wouldn't break anyway because getSetCCInverse is resilient to incorrectly assuming floating point (but not incorrectly assuming integer, which this patch fixes). This begs the question of why the extra parameter is necessary but that's a story for another day.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

static SDValue trySwapVSelectOperands(SDNode *N, SelectionDAG &DAG) {
   case ISD::FMUL:
   case ISD::FSUB:
   case ISD::FADD:
     break;
   }
   if (SelectA != SelectB.getOperand(0))
     return SDValue();
 
-  ISD::CondCode CC = cast<CondCodeSDNode>(SetCC->getOperand(2))->get();
-  auto InverseSetCC = DAG.getSetCC(
-      SDLoc(SetCC), SetCC.getValueType(), SetCC.getOperand(0),
-      SetCC.getOperand(1), ISD::getSetCCInverse(CC, SetCC.getValueType()));
+  ISD::CondCode CC = cast<CondCodeSDNode>(SetCC.getOperand(2))->get();
+  ISD::CondCode InverseCC =
+      ISD::getSetCCInverse(CC, SetCC.getOperand(0).getValueType());
+  auto InverseSetCC =
+      DAG.getSetCC(SDLoc(SetCC), SetCC.getValueType(), SetCC.getOperand(0),
+                   SetCC.getOperand(1), InverseCC);
 
   return DAG.getNode(ISD::VSELECT, SDLoc(N), NTy,
                      {InverseSetCC, SelectB, SelectA});
 }
 
 // vselect (v1i1 setcc) ->
 //     vselect (v1iXX setcc)  (XX is the size of the compared operand type)
 // FIXME: Currently the type legalizer can't handle VSELECT having v1i1 as

llvm/test/CodeGen/AArch64/sve-select.ll

; CHECK-NEXT:    ret
     %sel = select i1 %mask, <vscale x 16 x i1> %a, <vscale x 16 x i1> %b
     ret <vscale x 16 x i1> %sel
 }
 
 define <vscale x 4 x float> @select_f32_invert_fmul(<vscale x 4 x float> %a, <vscale x 4 x float> %b) #0 {
 ; CHECK-LABEL: select_f32_invert_fmul:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ptrue p0.s
-; CHECK-NEXT:    fcmne p0.s, p0/z, z0.s, #0.0
+; CHECK-NEXT:    fcmeq p1.s, p0/z, z0.s, #0.0
+; CHECK-NEXT:    not p0.b, p0/z, p1.b
 ; CHECK-NEXT:    fmul z0.s, p0/m, z0.s, z1.s
 ; CHECK-NEXT:    ret
   %p = fcmp oeq <vscale x 4 x float> %a, zeroinitializer
   %fmul = fmul <vscale x 4 x float> %a, %b
   %sel = select <vscale x 4 x i1> %p, <vscale x 4 x float> %a, <vscale x 4 x float> %fmul
   ret <vscale x 4 x float> %sel
 }
 
 define <vscale x 4 x float> @select_f32_invert_fadd(<vscale x 4 x float> %a, <vscale x 4 x float> %b) {
 ; CHECK-LABEL: select_f32_invert_fadd:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ptrue p0.s
-; CHECK-NEXT:    fcmne p0.s, p0/z, z0.s, #0.0
+; CHECK-NEXT:    fcmeq p1.s, p0/z, z0.s, #0.0
+; CHECK-NEXT:    not p0.b, p0/z, p1.b
 ; CHECK-NEXT:    fadd z0.s, p0/m, z0.s, z1.s
 ; CHECK-NEXT:    ret
   %p = fcmp oeq <vscale x 4 x float> %a, zeroinitializer
   %fadd = fadd <vscale x 4 x float> %a, %b
   %sel = select <vscale x 4 x i1> %p, <vscale x 4 x float> %a, <vscale x 4 x float> %fadd
   ret <vscale x 4 x float> %sel
 }
 
-define <vscale x 4 x float> @select_f32_invert_fsub(<vscale x 4 x float> %a, <vscale x 4 x float> %b) {
+define <vscale x 4 x float> @select_f32_invert_fsub(<vscale x 4 x float> %a, <vscale x 4 x float> %b, <vscale x 4 x i32> %c) {
 ; CHECK-LABEL: select_f32_invert_fsub:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    ptrue p0.s
-; CHECK-NEXT:    fcmne p0.s, p0/z, z0.s, #0.0
+; CHECK-NEXT:    cmpne p0.s, p0/z, z2.s, #0
 ; CHECK-NEXT:    fsub z0.s, p0/m, z0.s, z1.s
 ; CHECK-NEXT:    ret
-  %p = fcmp oeq <vscale x 4 x float> %a, zeroinitializer
+  %p = icmp eq <vscale x 4 x i32> %c, zeroinitializer
   %fsub = fsub <vscale x 4 x float> %a, %b
   %sel = select <vscale x 4 x i1> %p, <vscale x 4 x float> %a, <vscale x 4 x float> %fsub
   ret <vscale x 4 x float> %sel
 }
 
 define <vscale x 4 x float> @select_f32_no_invert_op_lhs(<vscale x 4 x float> %a, <vscale x 4 x float> %b) {
 ; CHECK-LABEL: select_f32_no_invert_op_lhs:
 ; CHECK:       // %bb.0: