[PATCH] D93622: [ARM] Extend lowering for i64 reductions

Mon Jan 4 03:57:14 PST 2021

SjoerdMeijer accepted this revision.
SjoerdMeijer added inline comments.
This revision is now accepted and ready to land.

================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:14968
+    EVT AVT = A.getValueType();
+    if (AVT.getSizeInBits() != 128)
+      A = DAG.getNode(ExtendCode, dl,
----------------
dmgreen wrote:
> SjoerdMeijer wrote:
> > I guess it would be better to replace 128 with a TTI call to query the size of vector registers? I forgot which hook that is though.
> I don't know if anything here can check TTI, nothing seems to in this file. I have changed it to use is128BitVector, which seems to be more common, and moved the comment closer which explains about non-legal types. I would expect readers to understand the size of a legal vector is 128 bits in MVE.
Ah yeah, forgot about that, but `is128BitVector()` looks a lot better anyway!

LGTM

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

https://reviews.llvm.org/D93622