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

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

The idea to combine the two switches sounds good if we can. It is a bit hard to follow if this would always be valid for all truncates through a bitcast. We should be protected against most of that because I don't think it can currently come up. Unfortunately that can make testing it difficult too.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17843
+  bool IsLittleEndian = DAG.getDataLayout().isLittleEndian();
+  if (IsLittleEndian && VT.isSimple() && VT.is64BitVector() &&
+      VT.getVectorNumElements() >= 2) {
----------------
mingmingl wrote:
> nit pick about style:
> 
> It's more idiomatic to bail out early, see https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
We only generate AArch64ISD::UZP1 from certain types, which will always be simple. I believe that because we only generate them post-lowering, we can currently rely on the truncates begin legal types too, but that might not always be true if things change.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17878
+    SDValue SourceOp1 = getSourceOp(Op1);
+    EVT DestVT = VT;
+
----------------
DestVT == VT == ResVT


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17580
+  // Use index 0 as ResNo directly since we know that N is a uzp1 DAG node.
+  EVT VT = N->getValueType(0);
+
----------------
VT can be replaced by ResVT.


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

https://reviews.llvm.org/D133850