[PATCH] D95551: [ARM] One-off identity shuffle

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:8246
+    int NonUndef = 0;
+    for (int i = 0, NumMaskElts = Mask.size(); i < NumMaskElts; ++i) {
+      if (Mask[i] == -1)
----------------
SjoerdMeijer wrote:
> nit: shorter is:
> 
>   for (int i = 0; i < Mask.size(); ++i)
I believe this is how llvm specifies this should be. So we only evaluate Mask.size() once. Apparently I got it from here:
https://github.com/llvm/llvm-project/blob/73aa09704a4c85b097d5fab986ead27092ecc9f7/llvm/lib/IR/Instructions.cpp#L2131


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:8247
+    for (int i = 0, NumMaskElts = Mask.size(); i < NumMaskElts; ++i) {
+      if (Mask[i] == -1)
+        continue;
----------------
SjoerdMeijer wrote:
> Do we have a test for this? Might have missed it, but didn't see one I think.
For undef elements? Yeah they will be tested as a part of the vld3 and shuffle3steptype tests I believe, but I'll make sure there are specific tests.


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

https://reviews.llvm.org/D95551