[PATCH] D139068: [AArch64][SVE] Allow to lower WHILEop operations with constant operands to PTRUE

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-while.ll:87
+; CHECK-NEXT:    ret
+  %out = call <vscale x 2 x i1> @llvm.aarch64.sve.whilele.nxv2i1.i64(i64 0, i64 3)
+  ret <vscale x 2 x i1> %out
----------------
I guess this doesn't match:

  ptrue p0.d, vl4

because `NumActiveElems.getZExtValue() <= (MinSVEVectorSize / ElementSize)` evaluates to false.

Can you clarify the reason these intrinsics don't fold in the name of these tests (e.g. `whilele_d_ii_dont_fold_to_ptrue_larger_than_minvec`) and/or add a comment to explain it?


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-while.ll:101
+
+define <vscale x 16 x i1> @whilele_b_ii2() {
+; CHECK-LABEL: whilele_b_ii2:
----------------
Same question here: rename this to `whilele_b_ii_dont_fold_to_ptrue_nonexistent_vl9` or something?


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-while.ll:121
+
+define <vscale x 16 x i1> @whilele_b_ii4() {
+; CHECK-LABEL: whilele_b_ii4:
----------------
same question about renaming this test, e.g. `whilele_b_ii_dont_fold_to_ptrue_overflow` ?


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-while.ll:270
+entry:
+  %out = call <vscale x 16 x i1> @llvm.aarch64.sve.whilelo.nxv16i1.i32(i32 4294967295, i32 6)
+  ret <vscale x 16 x i1> %out
----------------
For whilelo (unsigned), this is the same as -1, and is therefore not much different than the test above.
I would suggest keeping this test (because it's unsigned) and removing the test above


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-while.ll:410
+entry:
+  %out = call <vscale x 16 x i1> @llvm.aarch64.sve.whilels.nxv16i1.i32(i32 4294967295, i32 6)
+  ret <vscale x 16 x i1> %out
----------------
For whilels (unsigned), this is the same as `-1`, and is therefore not much different than the test above.
I would suggest keeping this test (because it's unsigned) and removing the test above.


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

https://reviews.llvm.org/D139068