[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 accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

Thanks. With a couple more nitpicks, this LGTM.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18077-18084
+  if (!SourceOp0 || !SourceOp1)
+    return SDValue();
+
+  if (SourceOp0.getValueType() != SourceOp1.getValueType())
+    return SDValue();
+
+  if (!SourceOp0.getValueType().isSimple())
----------------
These can be combined into a single if.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18104
+  SDValue UzpOp1 = DAG.getNode(ISD::BITCAST, DL, ResultTy, SourceOp1);
+  assert(UzpOp0.getValueType() == UzpOp1.getValueType());
+  SDValue UzpResult =
----------------
In llvm it is customary to add messages to asserts. In this case it is likely correct by construction, as we have just created the two ops with the same type above.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:18121
+  default:
+    llvm_unreachable("should be one of {v2i32, v4i16, v8i8}");
+  }
----------------
should -> Should


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

https://reviews.llvm.org/D133850