[PATCH] D121968: [AArch64] Fix incorrect getSetCCInverse usage within trySwapVSelectOperands.

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17523
+      DAG.getSetCC(SDLoc(SetCC), SetCC.getValueType(), SetCC.getOperand(0),
+                   SetCC.getOperand(1), ISD::getSetCCInverse(CC, NTy));
 
----------------
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.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121968/new/

https://reviews.llvm.org/D121968