> On 15 Jul 2024, at 5:18 pm, Jakub Jelinek <ja...@redhat.com> wrote: > > External email: Use caution opening links or attachments > > > On Mon, Jul 15, 2024 at 12:39:22AM +0000, Kugan Vivekanandarajah wrote: >> OMP safelen handling is assigning backend provided max as an int even when >> the pragma didn’t provide one. As a result, vectoriser is rejecting SVE >> modes while comparing poly_int with the safelen. >> >> That is, for the attached test case, omp_max_vf gets [16, 16] from the >> backend. This then becomes 16 as omp safelen is an integer. When vectoriser >> compares the potential vector mode with maybe_lt (max_vf, min_vf)) , this >> would fail resulting in any SVE vector mode being selected. >> >> One suggestion there was to set safelen to INT_MAX when OMP pragma does not >> provide safely explicitly. > > This is wrong. The reason why safelen is set to that sctx.max_vf is that if > there are any "omp simd array" arrays, sctx.max_vf is their size. > The code you're touching has a comment that explains it even: > /* If max_vf is non-zero, then we can use only a vectorization factor > up to the max_vf we chose. So stick it into the safelen clause. */ > if (maybe_ne (sctx.max_vf, 0U)) > > If sctx.max_vf is 0, there were no "omp simd array" arrays emitted and so > OMP_CLAUSE_SAFELEN isn't set. > The vectorizer can only shrink the arrays, not grow them and that is why > there is this limit. > > Now, I think even SVE has a limit, which is not a scalar constant but > poly_int, so I think in that case you need to arrange for the size of the > arrays to be POLY_INT_CST as well and use that as a limit. > Now, the clause argument itself at least in the OpenMP standard needs to be an > integer constant (if provided), because the proposals to extend it for the > SVE-like arches (aarch64, RISC-V) have not been voted in I believe. > So, there is a question what to do if user specifies safelen (32) or > something similar. > But if the user didn't specify it (so it is effectively infinitity), then > yes, it might be ok to set it to some POLY_INT_CST representing the sizes of > the arrays and tweak the loop safelen so that it can represent those.
Thanks for the explanation. Does that mean: 1. We change loop->safelen to poly_int 2. Modify the apply_safelen to account for the poly_int. I am attaching an RFC patch for your reference. Thanks, Kugan Signed-off-by: Kugan Vivekanandarajah <kvivekana...@nvidia.com> > >> PR middle-end/114635 >> PR 114635 >> >> gcc/ChangeLog: >> >> * omp-low.cc (lower_rec_input_clauses): Set INT_MAX >> when safelen is not provided instead of using backend >> provided safelen. >> >> gcc/testsuite/ChangeLog: >> >> * c-c++-common/pr114635-1.cpp: New test. >> * c-c++-common/pr114635-2.cpp: New test. >> >> Signed-off-by: Kugan Vivekanandarajah <kvivekana...@nvidia.com> > > Jakub
0001-safelen_v2.patch
Description: 0001-safelen_v2.patch
