[Openmp-commits] [openmp] [Libomptarget] Handle dynamic stack sizes for AMD COV5 (PR #72606)

[Joseph Huber via Openmp-commits]

jhuber6 wrote:

> It's really difficult to work out what's going on here from the diff.
> 
> I think you've rolled a change in default from 16k to 1k in along with the rest of it and got a branch inverted but it's hard to tell.
> 
> What we used to have is the compiler emitted a guess at the stack size needed. Now it just punts with "no idea, good luck runtime", in which case we assume 1k and fall over if that's no good? This behaviour is pretty weird - if the compiler can't tell how much stack to allocate, the chances that the single right number for all the difficult kernels is in an _environment variable_ seems pretty remote.

I talked a bit with Matt and reduced it, but I can keep it at 16K if needed.
 
> I think we should go with more sensible semantics or be much clearer with the naming of of things here. Something like:
> 
> `bool kernel_metadata_missing_stack_size()` `uint64_t guess_stack_size_for_missing_metadata_from_environment_variable()`

This more or less mimics the handling of the Nvidia stack. I believe the Nvidia stack defaults to 1024 or something in all cases if it wasn't statically determined. If we want to rework that I'd suggest it's in a follow-up patch that handles both cases.
 
> For more sensible semantics, we know the upper bound on how much stack size we could give a kernel, and we know that the compiler failing to determine a stack size means the application is doing something exciting with control flow, so I think we should (optionally warn and) default to the maximum. That is, your complicated kernel doing stuff we failed to analyse gets to run slowly, unless you annotate it somehow.
> 
> Further, this single environment variable to set all stack sizes for all unknown things is just a dreadful construct and I recommend should ignore it entirely.

Somewhat orthogonal to this as a bug-fix, right now it will be zero, which is pretty much guaranteed failure. I'd prefer to fix the stack handling in a more unified way later.

https://github.com/llvm/llvm-project/pull/72606