On 08/03/2024 10:16, Thomas Schwinge wrote:
Hi!
So, attached here is now a different patch
"GCN: The original meaning of 'GCN_SUPPRESS_HOST_FALLBACK' isn't applicable
(non-shared memory system)",
that takes a different approach re clarifying the two orthogonal aspects
that the 'GCN_SUPPRESS_HOST_FALLBACK' environment variable controls:
(a) the *original* meaning via 'HSA_SUPPRESS_HOST_FALLBACK', and
(b) the *additional*/*new* meaning to report as fatal certain errors
during device probing.
As you requested, (b) remains as it is (with just the diagnostic message
clarified). Re (a):
On 2024-03-07T14:37:10+0100, I wrote:
On 2024-03-07T12:43:07+0100, Tobias Burnus <tbur...@baylibre.com> wrote:
Thomas Schwinge wrote:
[...] libgomp GCN plugin 'GCN_SUPPRESS_HOST_FALLBACK' [...]
[...] originates in the libgomp HSA plugin, where the idea was -- in my
understanding -- that you wouldn't have device code available for all
'fn_ptr's, and in that case transparently (shared-memory system!) do
host-fallback execution. Or, with 'GCN_SUPPRESS_HOST_FALLBACK' set,
you'd get those diagnosed.
This has then been copied into the libgomp GCN plugin (see above).
However, is it really still applicable there; don't we assume that we're
generating device code for all relevant functions?
And, one step back: how is (the original meaning of)
'suppress_host_fallback = false' even supposed to work on non-shared
memory systems as currently implemented by the libgomp GCN plugin?
[...] this whole concept of dynamic plugin-level
host-fallback execution being in conflict with our current non-shared
memory system configurations?
I therefore suggest to get rid of (a).
OK to push?
I wasn't aware that things could be broken when fallback-suppression
*wasn't* set. I agree that we don't need that "feature".
As far as I knew this feature was merely an older implementation of the
now-standard OMP_TARGET_OFFLOAD=mandatory with the additional advantage
that we could make it do whatever we want for our test and debug needs
(i.e. no target independent "smarts").
This patch looks good, thanks.
Andrew
