[PATCH] D143283: [AArch64][SVE]: custom lower AVGFloor/AVGCeil.

[Dave Green via Phabricator via llvm-commits]

dmgreen accepted this revision.
dmgreen added a comment.

I think it's worth adding test for both the ashr and lshr versions, but otherwise I think this LGTM. Thanks



================
Comment at: llvm/test/CodeGen/AArch64/sve-hadd.ll:166
   %m = add nsw <vscale x 4 x i64> %s0s, %s1s
-  %s = lshr <vscale x 4 x i64> %m, shufflevector (<vscale x 4 x i64> insertelement (<vscale x 4 x i64> poison, i64 1, i32 0), <vscale x 4 x i64> poison, <vscale x 4 x i32> zeroinitializer)
+  %s = ashr <vscale x 4 x i64> %m, shufflevector (<vscale x 4 x i64> insertelement (<vscale x 4 x i64> poison, i64 1, i32 0), <vscale x 4 x i64> poison, <vscale x 4 x i32> zeroinitializer)
   %s2 = trunc <vscale x 4 x i64> %s to <vscale x 4 x i32>
----------------
Is there a lshr version of this one?  It would be good to have some that are "full width" and use lshr.
As instcombine will convert all the ashr to lshr it might be best to make sure there are tests for all the functions that were changed.


================
Comment at: llvm/test/CodeGen/AArch64/sve-hadd.ll:904
 
 define <vscale x 16 x i8> @rhadds_v16i8(<vscale x 16 x i8> %s0, <vscale x 16 x i8> %s1) {
+; SVE-LABEL: rhadds_v16i8:
----------------
hassnaa-arm wrote:
> here is a transformation proof for this case:
> https://alive2.llvm.org/ce/z/vPJi6R
> @dmgreen 
I think that's another case. This one would be https://alive2.llvm.org/ce/z/tp5NmX. From what I can tell they all look OK, according to alive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143283