On Sat, Nov 02, 2024 at 03:53:34PM +0000, Prathamesh Kulkarni wrote:
> The attached patch adds a new bitfield needs_max_vf_lowering to loop, and
> sets that in expand_omp_simd for loops that need delayed lowering of
> safelen and omp simd arrays. The patch defines a new macro
> OMP_COMMON_MAX_VF (arbitrarily set to 16), as a placeholder value for
> max_vf (instead of INT_MAX), and is later replaced by appropriate max_vf
> during omp_adjust_max_vf pass. Does that look OK ?
No.
The thing is, if user doesn't specify safelen, it defaults to infinity
(which we represent as INT_MAX), if user specifies it, then that is the
maximum for it (currently in OpenMP specification it is just an integral
value, so can't be a poly int).
And then the lowering uses the max_vf as another limit, what the hw can do
at most and sizes the magic arrays with it. So, one needs to use minimum
of what user specified and what the hw can handle.
So using 16 as some magic value is just wrong, safelen(16) can be specified
in the source as well, or safelen(8), or safelen(32) or safelen(123).
Thus, the fact that the hw minimum hasn't been determined yet needs to
be represented in some other flag, not in loop->safelen value, and before
that is determined, loop->safelen should then represent what the user wrote
(or was implied) and the later pass should use minimum from loop->safelen
and the picked hw maximum. Of course if the picked hw maximum is
POLY_INT-ish, the big question is how to compare that against the user
supplied integer value, either one can just handle the INT_MAX (aka
infinity) special case, or say query the backend on what is the maximum
value of the POLY_INT at runtime and only use the POLY_INT if it is always
known to be smaller or equal to the user supplied safelen.
Another thing (already mentioned in the thread Andrew referenced) is that
max_vf is used in two separate places. One is just to size of the magic
arrays and one of the operands of the minimum (the other is user specified
safelen). In this case, it is generally just fine to pick later value than
strictly necessary (as long as it is never larger than user supplied
safelen).
The other case is simd modifier on schedule clause. That value should
better be the right one or slightly larger, but not too much.
I think currently we just use the INTEGER_CST we pick as the maximum, if
this sizing is deferred, maybe it needs to be another internal function that
asks the value (though, it can refer to a loop vf in another function, which
complicates stuff).
Regarding Richi's question, I'm afraid the OpenMP simd loop lowering can't
be delayed until some later pass.
Jakub
