[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Dylan Fleming via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGef198cd99e6b: [SVE] Remove usage of getMaxVScale for 
AArch64, in favour of IR Attribute (authored by DylanFleming-arm).

Changed prior to commit:
  https://reviews.llvm.org/D106277?vs=366001&id=366885#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll

Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -12,7 +12,7 @@
 ; that we can use gather instructions with the correct offsets, taking
 ; vscale into account.
 
-define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) {
+define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) #0 {
 ; CHECK-LABEL: @widen_ptr_phi_unrolled(
 ; CHECK:   vector.body:
 ; CHECK-NEXT:[[POINTER_PHI:%.*]] = phi i32* [ %c, %vector.ph ], [ %[[PTR_IND:.*]], %vector.body ]
@@ -122,7 +122,7 @@
 ; because it is stored to memory.
 ;
 
-define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) {
+define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) #0 {
 ; CHECK-LABEL: @pointer_iv_mixed(
 ; CHECK: vector.body
 ; CHECK:   %[[IDX:.*]] = phi i64 [ 0, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -170,7 +170,7 @@
   ret i32 %tmp5
 }
 
-
+attributes #0 = { vscale_range(0, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -49,7 +49,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" }
+attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -mtriple aarch64-linux-gnu -mattr=+sve -loop-vectorize -scalable-vectorization=on -dce -instcombine -S <%s | FileCheck %s
 
-define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) {
+define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_i32(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -27,7 +27,7 @@
   ret void
 }
 
-define void @stride7_f64(double* noalias nocapture %dst, i64 %n) {
+define void @stride7_f64(double* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -55,7 +55,7 @@
 }
 
 
-define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) {
+define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) #0 {
 ; CHECK-LABEL: @cond_stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[MASK:.*]] = icmp ne 
@@ -90,7 +90,7 @@
   ret void
 }
 
-
+attributes #0 = { vscale_

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

Please remember to create a more representative commit message as the patch no 
longer removes getMaxVScale.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Dylan Fleming via Phabricator via cfe-commits]

DylanFleming-arm updated this revision to Diff 366001.
DylanFleming-arm added a comment.

Added checks for MaxVScale > 0
Changed getMaxNumElemenets() to take Function* instead of Instruction*
Fixed clang-tidy warning

I haven't included a test for vscale_range(2, 0) here, as one was added in the 
meantime in commit fe6ae81 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll

Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -12,7 +12,7 @@
 ; that we can use gather instructions with the correct offsets, taking
 ; vscale into account.
 
-define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) {
+define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) #0 {
 ; CHECK-LABEL: @widen_ptr_phi_unrolled(
 ; CHECK:   vector.body:
 ; CHECK-NEXT:[[POINTER_PHI:%.*]] = phi i32* [ %c, %vector.ph ], [ %[[PTR_IND:.*]], %vector.body ]
@@ -122,7 +122,7 @@
 ; because it is stored to memory.
 ;
 
-define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) {
+define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) #0 {
 ; CHECK-LABEL: @pointer_iv_mixed(
 ; CHECK: vector.body
 ; CHECK:   %[[IDX:.*]] = phi i64 [ 0, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -170,7 +170,7 @@
   ret i32 %tmp5
 }
 
-
+attributes #0 = { vscale_range(0, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -49,7 +49,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" }
+attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -mtriple aarch64-linux-gnu -mattr=+sve -loop-vectorize -scalable-vectorization=on -dce -instcombine -S <%s | FileCheck %s
 
-define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) {
+define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_i32(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -27,7 +27,7 @@
   ret void
 }
 
-define void @stride7_f64(double* noalias nocapture %dst, i64 %n) {
+define void @stride7_f64(double* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -55,7 +55,7 @@
 }
 
 
-define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) {
+define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) #0 {
 ; CHECK-LABEL: @cond_stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[MASK:.*]] = icmp ne 
@@ -90,7 +90,7 @@
   

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h:134
+  unsigned getMaxNumElements(ElementCount VF,
+ const Instruction *I = nullptr) const {
 if (!VF.isScalable())

Can this parameter be a `Function*`? given there's no real link between this 
function and LLVM Instructions.





Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h:142-143
+ ->getFnAttribute(Attribute::VScaleRange)
+ .getVScaleRangeArgs()
+ .second;
+}

This can return `0` implying there is no know maximum.  With the current code 
this means `0` will be returned instead of a sensible default.



Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5620
+TheFunction->getFnAttribute(Attribute::VScaleRange);
+MaxVScale = VScaleRangeAttr.getVScaleRangeArgs().second;
+  }

I think you only want to set `MaxVScale` when 
`VScaleRangeAttr.getVScaleRangeArgs().second` is non-zero.

Given this and the above similar comment perhaps there's need for extra tests 
that cover `vscale_range(2,0)` for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Fraser Cormack via Phabricator via cfe-commits]

frasercrmck added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.h:100
+  Optional>
+  getVScaleRange(const LangOptions &LangOpts) const;
+

This clang-tidy warning needs satisfied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Fraser Cormack via Phabricator via cfe-commits]

frasercrmck added a comment.

In D106277#2913136 , @paulwalker-arm 
wrote:

> @craig.topper can you share RISCV plans around supporting vscale_range?  In 
> essence we'd like to deprecate the TTI method and have LLVM IR contain all 
> relevant information when is comes to interpreting vscale.
>
> Currently the usage is minimal and so checking both interfaces is not too bad 
> but they'll come a point when there's no TTI available and then only the side 
> supporting vscale_range can be considered.  There's also the LTO side of 
> things where relying on opt/llc flags to set register widths becomes fragile.

As it happens I was playing around with adding support for that today at least 
to get us started. I was going to put something up for review based on this 
patch, assuming this gets merged first.

However in the TTI method we're currently making use of some `RISCVSubtarget` 
properties to help refine the vscale range, so we'll need to think about how 
we'll deal with those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

@craig.topper can you share RISCV plans around supporting vscale_range?  In 
essence we'd like to deprecate the TTI method and have LLVM IR contain all 
relevant information when is comes to interpreting vscale.

Currently the usage is minimal and so checking both interfaces is not too bad 
but they'll come a point when there's no TTI available and then only the side 
supporting vscale_range can be considered.  There's also the LTO side of things 
where relying on opt/llc flags to set register widths becomes fragile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Dylan Fleming via Phabricator via cfe-commits]

DylanFleming-arm updated this revision to Diff 362020.
DylanFleming-arm added a comment.

Rebased onto main, updated newly added AArch64 getMaxVScale usages to use IR 
attribute instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll

Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -12,7 +12,7 @@
 ; that we can use gather instructions with the correct offsets, taking
 ; vscale into account.
 
-define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) {
+define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) #0 {
 ; CHECK-LABEL: @widen_ptr_phi_unrolled(
 ; CHECK:   vector.body:
 ; CHECK-NEXT:[[POINTER_PHI:%.*]] = phi i32* [ %c, %vector.ph ], [ %[[PTR_IND:.*]], %vector.body ]
@@ -122,7 +122,7 @@
 ; because it is stored to memory.
 ;
 
-define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) {
+define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) #0 {
 ; CHECK-LABEL: @pointer_iv_mixed(
 ; CHECK: vector.body
 ; CHECK:   %[[IDX:.*]] = phi i64 [ 0, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -170,7 +170,7 @@
   ret i32 %tmp5
 }
 
-
+attributes #0 = { vscale_range(0, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -49,7 +49,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" }
+attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -mtriple aarch64-linux-gnu -mattr=+sve -loop-vectorize -scalable-vectorization=on -dce -instcombine -S <%s | FileCheck %s
 
-define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) {
+define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_i32(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -27,7 +27,7 @@
   ret void
 }
 
-define void @stride7_f64(double* noalias nocapture %dst, i64 %n) {
+define void @stride7_f64(double* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -55,7 +55,7 @@
 }
 
 
-define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) {
+define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) #0 {
 ; CHECK-LABEL: @cond_stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[MASK:.*]] = icmp ne 
@@ -90,7 +90,7 @@
   ret void
 }
 
-
+attributes #0 = { vscale_range(0, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/Loop

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

You'll want to rebase your patch on top of latest main, since D105432 
 moved around the call to `getMaxVScale()` in 
AArch64TargetTransformInfo.cpp.




Comment at: clang/lib/Basic/Targets/AArch64.cpp:433
+  }
+  if (hasFeature("sve")) {
+return std::pair(0, 16);

nit: unnecessary curly braces.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:499
 
   // Add vscale attribute if appropriate.
+  Optional> VScaleRange =

nit: `vscale_range`



Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1424
   ElementCount LegalVF = LT.second.getVectorElementCount();
-  Optional MaxNumVScale = getMaxVScale();
+  Optional MaxNumVScale;
+  if (I->getFunction()->hasFnAttribute(Attribute::VScaleRange)) {

This can drop the Optional now.

I also think asserting the attribute must be set is a bit of a strong 
requirement? Maybe we can return an Invalid instead if the attribute is not set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Dylan Fleming via Phabricator via cfe-commits]

DylanFleming-arm updated this revision to Diff 361675.
DylanFleming-arm added a comment.

Added getVScaleRange interface to TargetInfo and removed related AArch64 
specific code from CodeGenFunction.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll

Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -12,7 +12,7 @@
 ; that we can use gather instructions with the correct offsets, taking
 ; vscale into account.
 
-define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) {
+define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) #0 {
 ; CHECK-LABEL: @widen_ptr_phi_unrolled(
 ; CHECK:   vector.body:
 ; CHECK-NEXT:[[POINTER_PHI:%.*]] = phi i32* [ %c, %vector.ph ], [ %[[PTR_IND:.*]], %vector.body ]
@@ -122,7 +122,7 @@
 ; because it is stored to memory.
 ;
 
-define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) {
+define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) #0 {
 ; CHECK-LABEL: @pointer_iv_mixed(
 ; CHECK: vector.body
 ; CHECK:   %[[IDX:.*]] = phi i64 [ 0, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -170,7 +170,7 @@
   ret i32 %tmp5
 }
 
-
+attributes #0 = { vscale_range(0, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -mtriple aarch64-linux-gnu -mattr=+sve -loop-vectorize -scalable-vectorization=on -dce -instcombine -S <%s | FileCheck %s
 
-define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) {
+define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_i32(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -27,7 +27,7 @@
   ret void
 }
 
-define void @stride7_f64(double* noalias nocapture %dst, i64 %n) {
+define void @stride7_f64(double* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -55,7 +55,7 @@
 }
 
 
-define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) {
+define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) #0 {
 ; CHECK-LABEL: @cond_stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[MASK:.*]] = icmp ne 
@@ -90,7 +90,7 @@
   ret void
 }
 
-
+attributes #0 = { vscale_range(0, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
@@ -59,7 +59,7 @@
   ret void
 }
 
-attributes #0 = { "target-features"="+neon,+sve" }
+attributes #0 = { "target-features"="+neon,+sve" vscale_range(0, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
==

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:507
+CurFn->addFnAttr(
+llvm::Attribute::getWithVScaleRangeArgs(getLLVMContext(), 0, 16));
   }

Thinking about this more, maybe these values should be returned from a new 
interface on TargetInfo? Maybe using Optional so you can default to None for 
other targets? This way you don't have AArch64 specific numbers hardcoded in a 
file that doesn't belong to AArch64? But maybe the damage was already done with 
the "/ 128" in the ArmSveVectorBits code above. Maybe you can unify both into a 
new TargetInfo function? Does TargetInfo have access to LangOpts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Dylan Fleming via Phabricator via cfe-commits]

DylanFleming-arm updated this revision to Diff 360879.
DylanFleming-arm added a comment.

Removed changes to RiscV code
Added check that target isAArch64 before adding default value vscale_range 
attribute


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll

Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -12,7 +12,7 @@
 ; that we can use gather instructions with the correct offsets, taking
 ; vscale into account.
 
-define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) {
+define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) #0 {
 ; CHECK-LABEL: @widen_ptr_phi_unrolled(
 ; CHECK:   vector.body:
 ; CHECK-NEXT:[[POINTER_PHI:%.*]] = phi i32* [ %c, %vector.ph ], [ %[[PTR_IND:.*]], %vector.body ]
@@ -122,7 +122,7 @@
 ; because it is stored to memory.
 ;
 
-define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) {
+define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) #0 {
 ; CHECK-LABEL: @pointer_iv_mixed(
 ; CHECK: vector.body
 ; CHECK:   %[[IDX:.*]] = phi i64 [ 0, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -170,7 +170,7 @@
   ret i32 %tmp5
 }
 
-
+attributes #0 = { vscale_range(0, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -mtriple aarch64-linux-gnu -mattr=+sve -loop-vectorize -scalable-vectorization=on -dce -instcombine -S <%s | FileCheck %s
 
-define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) {
+define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_i32(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -27,7 +27,7 @@
   ret void
 }
 
-define void @stride7_f64(double* noalias nocapture %dst, i64 %n) {
+define void @stride7_f64(double* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -55,7 +55,7 @@
 }
 
 
-define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) {
+define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) #0 {
 ; CHECK-LABEL: @cond_stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[MASK:.*]] = icmp ne 
@@ -90,7 +90,7 @@
   ret void
 }
 
-
+attributes #0 = { vscale_range(0, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
@@ -59,7 +59,7 @@
   ret void
 }
 
-attributes #0 = { "target-features"="+neon,+sve" }
+attributes #0 = { "target-features"="+neon,+sve" vscale_range(0, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
+++ llvm/

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:504
  VScale, VScale));
+  } else if (getContext().getTargetInfo().hasFeature("sve")) {
+CurFn->addFnAttr(

Is there any way we can check that the target is ARM before checking an "sve" 
on a target independent interface? If any other target uses the name "sve" this 
will trigger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:119-131
-Optional RISCVTTIImpl::getMaxVScale() const {
-  // There is no assumption of the maximum vector length in V specification.
-  // We use the value specified by users as the maximum vector length.
-  // This function will use the assumed maximum vector length to get the
-  // maximum vscale for LoopVectorizer.
-  // If users do not specify the maximum vector length, we have no way to
-  // know whether the LoopVectorizer is safe to do or not.

bsmith wrote:
> I'm not sure that RISCV have made a commitment to use the vscale_range 
> attribute yet have they? In either case I think they should be involved in a 
> change like this.
Perhaps it's worth this patch not removing getMaxVScale just yet but rather 
just AArch64's implementation?  There would only need to be a minor change to 
LoopVectorize.cpp along the lines of `if (!MaxVScale && 
TheFunction->hasFnAttribute(Attribute::VScaleRange...`.  That way getMaxVScale 
can be removed if/when no one needs it, which I hope is not too far away.

If there is agreement to remove it then I imagine code similar to what you've 
done for SVE in CodeGenFunction.cpp will be needed for RISCV otherwise the 
patch will cause a regression in functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Bradley Smith via Phabricator via cfe-commits]

bsmith added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:505-506
+  } else if (getContext().getTargetInfo().hasFeature("sve")) {
+CurFn->addFnAttr(
+llvm::Attribute::getWithVScaleRangeArgs(getLLVMContext(), 0, 16));
   }

paulwalker-arm wrote:
> bsmith wrote:
> > Is this really what we want? Won't this enable fixed length codegen all of 
> > the time?
> Fixed length codegen is tied to the minimum `vscale` value, so by using `0` 
> here means nothing is known about the minimum `vscale` and thus fixed length 
> codegen will be restricted to 128bit as is the case when no attribute is 
> specified.
Ah right ok, I'd missed that detail. Ignore me then!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:505-506
+  } else if (getContext().getTargetInfo().hasFeature("sve")) {
+CurFn->addFnAttr(
+llvm::Attribute::getWithVScaleRangeArgs(getLLVMContext(), 0, 16));
   }

bsmith wrote:
> Is this really what we want? Won't this enable fixed length codegen all of 
> the time?
Fixed length codegen is tied to the minimum `vscale` value, so by using `0` 
here means nothing is known about the minimum `vscale` and thus fixed length 
codegen will be restricted to 128bit as is the case when no attribute is 
specified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Bradley Smith via Phabricator via cfe-commits]

bsmith added a reviewer: paulwalker-arm.
bsmith added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:505-506
+  } else if (getContext().getTargetInfo().hasFeature("sve")) {
+CurFn->addFnAttr(
+llvm::Attribute::getWithVScaleRangeArgs(getLLVMContext(), 0, 16));
   }

Is this really what we want? Won't this enable fixed length codegen all of the 
time?



Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:119-131
-Optional RISCVTTIImpl::getMaxVScale() const {
-  // There is no assumption of the maximum vector length in V specification.
-  // We use the value specified by users as the maximum vector length.
-  // This function will use the assumed maximum vector length to get the
-  // maximum vscale for LoopVectorizer.
-  // If users do not specify the maximum vector length, we have no way to
-  // know whether the LoopVectorizer is safe to do or not.

I'm not sure that RISCV have made a commitment to use the vscale_range 
attribute yet have they? In either case I think they should be involved in a 
change like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits