[PATCH] D75580: [llvm][CodeGen][SVE] Implement IR intrinsics for gather prefetch.

Francesco Petrogalli via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 13 11:50:10 PDT 2020


fpetrogalli marked 2 inline comments as done.
fpetrogalli added a comment.

In D75580#1921350 <https://reviews.llvm.org/D75580#1921350>, @andwar wrote:

> Thanks you for addressing my comments @fpetrogalli !
>
> A one final nice-to-have (not a blocker!). Could this snippet from `legalizeSVEGatherPrefetchOffsVec` be extracted into a separate function:
>
>   // Not an unpacked vector, bail out.
>   if (Offset.getValueType().getSimpleVT().SimpleTy != MVT::nxv2i32)
>     return SDValue();
>   
>   // Extend the unpacked offset vector to 64-bit lanes.
>   SDLoc DL(N);
>   Offset = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::nxv2i64, Offset);
>   
>
> and re-used by `performGatherLoadCombine` and `performScatterStoreCombine`? We can do it in a separate patch too.


Is it worth? The logic of these methods seems quite different to me. What performGatherLoadCombine` and `performScatterStoreCombine` do is "do something in any case, if the offset is unpacked, extend it".  What the method `legalizeSVEGatherPrefetchOffsVec` do is "do nothing, unless the offset is unpacked". At the end, what I can see that can be merged is the `Offset = DAG.getNode(...` invocation that insert the `ANY_EXTEND`. Not much that is worth factoring out.

Francesco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75580





More information about the llvm-commits mailing list