[PATCH] D77590: [ARM] MVE saturating truncates

[Dave Green via Phabricator via llvm-commits]

dmgreen marked 4 inline comments as done.
dmgreen added a comment.

Thanks for taking a look.

In D77590#1967680 <https://reviews.llvm.org/D77590#1967680>, @SjoerdMeijer wrote:

> About the tests: if I haven't missed anything, I don't see any VQMOVNT variants being generated?


Not with this one. I'll put up a review that transforms VMOVNT(VQMOVNB) -> VQMOVNT, once I have the tests.



================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:14796
+
+    // Create a VQMONVB, Signed extended in to the top half. That extend will
+    // hopefully be removed if only the bottom bits are demanded (though a
----------------
SjoerdMeijer wrote:
> Typo? `VQMONVB` -> `VQMOVB`?
> 
> Also: top half -> bottom half?
Yep. Will fix.

The "Signed extended in to the top half" means we signed extend the result of the VQMOVNB into the top half of the registers with the SIGN_EXTEND_INREG. i.e. we create a "bottom", but need to do something with the top half too.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.h:205
 
+      // MVE Saturating truncates
+      VQMOVNs,
----------------
SjoerdMeijer wrote:
> nit: perhaps a comment explaining more what the opcode is:
> 
>   // Vector (V) Saturating (Q) Move and Narrow (N),  signed/unsigned (s/u)
Like it.


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

https://reviews.llvm.org/D77590