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

Fri Mar 18 03:00:23 PDT 2022

paulwalker-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));

----------------
david-arm wrote:
> 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.
> 
Are yes, not sure what I was thinking. Thanks Dave.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121968