[PATCH] D63937: [ARM] MVE: allow soft-float ABI to pass vector types.

Mon Jul 1 05:33:06 PDT 2019

simon_tatham marked 2 inline comments as done.
simon_tatham added inline comments.

================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:268
     setOperationAction(ISD::STORE, VT, Legal);
+    setOperationAction(ISD::UNDEF, VT, Legal);

----------------
dmgreen wrote:
> What exactly does making UNDEF legal do? Is there a universal pattern to just ignore it? Why is that not the default?
The observable effect of having UNDEF //not// be legal is this kind of thing you can see in the existing expected results for `mve-fmath.ll`:
```
sqrt_float32_t:
@ %bb.0: @ %entry
  vsqrt.f32 s4, s0
  movs r0, #0
  vsqrt.f32 s6, s1
  vsqrt.f32 s8, s2
  vsqrt.f32 s10, s3
  vdup.32 q0, r0
  vmov.f32 s0, s4
  vmov.f32 s1, s6
  vmov.f32 s2, s8
  vmov.f32 s3, s10
  bx lr
```
in which the `movs r0,#0` and `vdup.32 q0,r0` pair are filling q0 with all zeroes, which is pointless because the next four instructions overwrite every part of q0 in any case. That happens because an ISD::UNDEF for that vector type is not regarded as legal, and the fallback lowering turns it into 'make a vector of zeroes'.

I have to suppose that UNDEF //is// legal by default and does the obvious thing – it's just that when we use the blunt instrument of `setAllExpand`, that sets it to `Expand` along with everything else, and we have to remember to put it back again.

================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:299
+    setOperationAction(ISD::BUILD_VECTOR, VT, Custom);
+    setOperationAction(ISD::UNDEF, VT, Legal);
   }
----------------
dmgreen wrote:
> Also if we make vector float and longs undef legal, should we be doing the same for integer vectors?
I //think// that's not necessary, because we didn't make it illegal in the first place by calling `setAllExpand`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63937