[PATCH] D133850: [AArch64] Improve codegen for "trunc <4 x i64> to <4 x i8>" for all cases

[Mingming Liu via Phabricator via llvm-commits]

mingmingl added a comment.

Thanks for working on this! I got my hands tight on working on this [1] but I'm more than glad to collaborate on the review side as well!

A high level question, https://reviews.llvm.org/D133280 has some other test cases for the same pattern, also on big-endian and little-endian systems to show the potential difference. It'd be great to generalize the current solution for those test cases. For simplicity, I'd probably start from solving the problem on little-endian first.

Also, I found it helpful to keep these in mind while working on an implementation

1. the caveats of `BITCAST` (across vectors, or across vectors and scalars)
2. memory layout of LLVM IR vectors

https://reviews.llvm.org/D94964 answers the above two questions perfectly.

[1] A preview of unfinished work in https://reviews.llvm.org/differential/diff/460138/ (not polished in terms of code style, and not generalized enough yet)



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17586-17591
+    EVT CastResultType = MVT::Other;
+
+    if (OrigOp0.getValueType().getSimpleVT().SimpleTy == MVT::v2i64)
+      CastResultType = MVT::v4i32;
+
+    if (CastResultType != MVT::Other) {
----------------
nit: 

If I read correctly, this uses `MVT::Other` as a sentinel.

To limit the scope of the problem to be solved, an alternative option (without multiplexing the semantics of `MVT::Other`)

```
// This won't generalize the solution as commented, but just to illustrate another way of limiting the scope
if OrinOp.getValueType().getSimpleVT() != MVT::v2i64
  return false;
```




================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17588
+
+    if (OrigOp0.getValueType().getSimpleVT().SimpleTy == MVT::v2i64)
+      CastResultType = MVT::v4i32;
----------------
nit:

Bail early (return false) if `OrigOp0.getValueType()` is not a simple type (`getSimpleVT()` will assert if `OrigOp0.getValueType()` is not a simple type.


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

https://reviews.llvm.org/D133850