[Openmp-commits] [PATCH] D147511: [OpenMP] Fix nextgen plugin behavior when passing negative thread_limit clause

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added a comment.

Apologies for the delay.

So, this is good and bad.

The bas part is that it actually doesn't matter if the value is "negative". The standard, at least as far as I can remember, doesn't specify if the value is supposed to be 32 bit signed or something else. Hence, `UINT_MAX` as thread limit should just be `UINT_MAX` and not `INT_MIN` which we handle differently than `UINT_MAX/2`.

The good part is that we can actually lower this upper bound, at least if it was not forced (which 5.2 or newer allow, IIRC). Thus, we can lower an unrealistic thread limit to something more meaningful if we want to. However, I don't see why we should make the cutoff at "signed bit set" and not earlier.

Overall, I'd like more reasoning for this threshold so we can potentially come up with a more meaningful way of determining the number of threads, e.g., by looking at the resources used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147511