[PATCH] [AArch64] Match interleaved memory accesses into ldN/stN instructions.

[James Molloy]

Hi Michael,

> For instance, functions like `isReInterleaveMask` look totally target
independent. Would it be possible to make a target-independent
implementation with a bunch of target hooks (like
`getInterleavedStoreIntrinsic(Factor, Size)`)? Then, ARM32 and ARM64
patches would differ only in those hooks, while the rest of the code would
be reused.

Chatting with Hao yesterday, The problem is not how to expose reuse, but
where in-tree to put the common, reusable parts. There's no obvious place
to hang it off at the moment.

Renato's suggested lib/CodeGen - is that a reasonable place?

I LGTM'd because I didn't see this problem being solved in a short time;
I'd have held off approving the ARM32 one until it was sorted though.

Cheers,

James

On Thu, 11 Jun 2015 at 21:37 Michael Zolotukhin <mzolotukhin at apple.com>
wrote:

> Hi Hao,
>
> Thanks for the patch. The general idea sounds good to me, but I have some
> questions regarding some implementation details. To address them, it would
> actually be helpful to see the ARM32 patch too, since I expect that a lot
> of the stuff can be reused, as James and Renato have already mentioned.
>
> For instance, functions like `isReInterleaveMask` look totally target
> independent. Would it be possible to make a target-independent
> implementation with a bunch of target hooks (like
> `getInterleavedStoreIntrinsic(Factor, Size)`)? Then, ARM32 and ARM64
> patches would differ only in those hooks, while the rest of the code would
> be reused.
>
>
> ================
> Comment at: lib/Target/AArch64/AArch64InterleavedAccess.cpp:146-148
> @@ +145,5 @@
> +                               unsigned &Index) {
> +  unsigned NumElts = Mask.size();
> +  if (NumElts < 2)
> +    return false;
> +
> ----------------
> ```
> if (Mask.size() < 2)
>   return false;
> ```
> ?
>
> ================
> Comment at: lib/Target/AArch64/AArch64InterleavedAccess.cpp:247
> @@ +246,3 @@
> +
> +    unsigned Index;
> +    if (!isDeInterleaveMaskOfFactor(Shuffles[i]->getShuffleMask(), Factor,
> ----------------
> Is there a need to redeclare `Index` here?
>
> ================
> Comment at: lib/Target/AArch64/AArch64InterleavedAccess.cpp:279
> @@ +278,3 @@
> +    ShuffleVectorInst *SV = Shuffles[i];
> +    unsigned Index = Indices[i];
> +
> ----------------
> One more redeclaration of `Index`.
>
> ================
> Comment at: lib/Target/AArch64/AArch64InterleavedAccess.cpp:351
> @@ +350,3 @@
> +
> +  SmallVector<Value *, 5> Ops;
> +
> ----------------
> Why `5`?
>
> http://reviews.llvm.org/D10335
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150612/482783fc/attachment.html>