[PATCH] D29466: [ARM/AArch64] Support wide interleaved accesses

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

Hi Matthew,

This change seems pretty straight forward. I only have some silly comments inline and a simple request: to create one additional test for three LD2s (even/odd extract from <24 x i32>) on each arch, just to make sure that it scales.

cheers,
--renato



================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:7281
 
+  // We can "legalize" wide vector types into multiple interleaved loads as
+  // long as the vector types are divisible by 128. The code below determines
----------------
I almost missed the modulo check above. To make it easier for posterity, I'd recommend move the 128 check after this comment.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:7414
 
+  // We can "legalize" wide vector types into multiple interleaved stores as
+  // long as the vector types are divisible by 128. The code below determines
----------------
Now that I see repeated, this could be a simple inline function?


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:7486
         }
+        // Note: If all elements in a chunk are undefs, StartMask=0! Note:
+        // Filling undef gaps with random elements is ok, since those elements
----------------
this comment change is unnecessary? was that clang-format?


https://reviews.llvm.org/D29466