[PATCH] D89831: [AArch64][SVE] Fix umin/umax lowering to handle out of range imm.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:3146
+    case MVT::i16:
+      ImmVal &= 0xFFFF;
+      break;
----------------
huihuiz wrote:
> sdesmalen wrote:
> > Maybe I'm missing something obvious, but I don't see why any masking is needed.
> > Is removing `ImmVal = ImmVal & 0xFF;` not sufficient?
> We need to apply masking "ImmVal & 0xFF" for i8 at least. Otherwise we got regression.
> 
> Take t.ll below, if we don't apply the mask for i8, then we generate "mov     z1.b, #-127 ".
> With the mask we generate "umax    z0.b, z0.b, #129".
> There are similar regression for i8 in range [128, 255]
> 
> ```
> define <vscale x 16 x i8> @umax_i8_large(<vscale x 16 x i8> %a) {
>   %elt = insertelement <vscale x 16 x i8> undef, i8 129, i32 0
>   %splat = shufflevector <vscale x 16 x i8> %elt, <vscale x 16 x i8> undef, <vscale x 16 x i32> zeroinitializer
>   %cmp = icmp ugt <vscale x 16 x i8> %a, %splat
>   %res = select <vscale x 16 x i1> %cmp, <vscale x 16 x i8> %a, <vscale x 16 x i8> %splat
>   ret <vscale x 16 x i8> %res
> }
> ```
To add to this, the SelectionDAG node for the operand looks something like the following: `nxv16i8 AArch64ISD::DUP(i32 -127)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89831