[PATCH] D129758: [AArch64][SVE] Lower DUPLANE128 to LD1RQD

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-perm-select.ll:592-594
   %1 = tail call fast <vscale x 2 x double> @llvm.vector.insert.nxv2f64.v2f64(<vscale x 2 x double> undef, <2 x double> <double 1.000000e+00, double 2.000000e+00>, i64 0)
   %2 = tail call fast <vscale x 2 x double> @llvm.aarch64.sve.dupq.lane.nxv2f64(<vscale x 2 x double> %1, i64 0)
   ret <vscale x 2 x double> %2
----------------
paulwalker-arm wrote:
> Based on the patch I think you should simplify all `ld1rq#` tests by removing the constants and just have the test load the data explicitly.  This will also help in the future if there turns out to be a better way to compute the constants vectors these tests are doing.  So for example:
> ```
> define dso_local <vscale x 2 x double> @dupq_ld1rqd_f64(ptr %a) {
>   %1 = load <2 x double>, ptr %a
>   %2 = tail call fast <vscale x 2 x double> @llvm.vector.insert.nxv2f64.v2f64(<vscale x 2 x double> undef, <2 x double> %1, i64 0)
>   %3 = tail call fast <vscale x 2 x double> @llvm.aarch64.sve.dupq.lane.nxv2f64(<vscale x 2 x double> %2, i64 0)
>   ret <vscale x 2 x double> %3
> }
> ``` 
> I also believe the tests are better placed in `sve-ld1r.ll`.
I'd like to backtrack slightly.  I still believe we want simpler tests added to sve-ld1r.ll for the new isel patterns. Then I guess these existing tests are required to show the need for the DAG combine. For this reason I think you want two patches, one for the isel then a second for the DAG combine


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129758