[PATCH] D98030: [IR] Add vscale_range IR function attribute

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c:10-14
+// CHECK-128: attributes #0 = { {{.*}} vscale_range(1,1) {{.*}} }
+// CHECK-256: attributes #0 = { {{.*}} vscale_range(2,2) {{.*}} }
+// CHECK-512: attributes #0 = { {{.*}} vscale_range(4,4) {{.*}} }
+// CHECK-1024: attributes #0 = { {{.*}} vscale_range(8,8) {{.*}} }
+// CHECK-2048: attributes #0 = { {{.*}} vscale_range(16,16) {{.*}} }
----------------
I'm happy with this but for information you could use the same trick as for the `llvm/test/Analysis/CostModel/AArch64/sve-fixed-length.ll` so that only a single CHECK, and CHECK-NOT line is required.


================
Comment at: llvm/include/llvm/IR/Attributes.h:944
 
+  /// Add an vscale_range attribute, using the representation returned by
+  /// Attribute.getIntValue().
----------------
a


================
Comment at: llvm/test/Verifier/vscale_range.ll:3-4
+
+; CHECK-NOT: 'vscale_range' minimum cannot be greater than maximum
+declare i8* @a(i32) vscale_range(1, 0)
+
----------------
I think attributes.ll is a better place for this test, along with a CHECK for it's expected output.

Kind of related, what is expected for `vscale_range(0,0)` and does that need a test (positive or negative)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98030