Thomas Schwinge wrote:
On 2025-05-12T17:03:29+0200, I wrote:
"Add effective-target 'offload_device_usm',
'libgomp.c-c++-common/target-usm-1.c'"
On top, we could then add the attached
"libgomp: Add a few more OpenMP/USM test cases"? These all PASS for GCN
gfx90a with 'HSA_XNACK=1'.
For the current 'libgomp.c-c++-common/map-arrayofstruct-2.c':
- No error if the default device uses self mapping
Special case: no device available as then the host is used.
- If a device is available, expect a mapping error.
The new 'usm' file uses 'requires unified_shared_memory' and
expects that all mappings are self-mappings
→ The current effective target "just" prevents running the code
again. Although with USM is not supported but a device is
available, the -usm file will run the host fallback while the
non-usm file will run on the device (and print the fail).
Thus, those cases aren't identical!
→ I think this should rather use 'requires self_maps' avoiding
the issue that 'requires unified_shared_memory' will use
copy-mapping as permitted.
The latter is probably not a real issue as GCC's default will
likely remain 'requires unified_shared_memory' → self-mapping,
unless a device will show up where we know that copying data
will (nearly) always be a win.
(Likewise for the other tests.)
* * *
To conclude:
Using 'requires self_maps' is surely cleaner as the tests seem
to strictly assume self mapping - however, no issue is expected
with (current + future) GCC default settings.
(Additionally, the way the effective target is written, USM
w/ copy mapping is recognized as not supported.)
Requires as effective target '!USM' will save some testing time
for users without devices (without test coverage loss) and also
for non-USM device owners, loosing the test coverage for the
host fallback. (However, there are enough non-offload users to
get that coverage later + it is not super interesting.)
Assuming that we don't mind the lost test coverage, I think I am
fine with the addition, still
(a) I'd prefer 'requires self_maps' - which is supported since GCC 15
(b) I am not happy with the effective target check name vs.
effect. IMHO it
- either should check only for USM availability
- or if it checks for 'unified_shared_memory' with self-mapping,
the target name should make this clear (e.g. 'usm_self_mapping').
When using 'self_maps', either check is fine - as the requirement
takes care of the self-mapping.
When keeping 'unified_shared_memory', I guess a 'usm' + self_mapping'
check is safer than a simple 'usm' check.
* * *
Note that for testsuite/libgomp.fortran/map-alloc-comp-9-usm.f90:
This testcase would have the best test coverage for
'usm' + copy-mapping
Thus, when you add a check for USM with self-mapping, the test
become almost completely pointless. - Thus, I surely do not want
to have any effective target on that file that only runs it when
USM is self mapping. An effective target like USM would be okay
(as the mainfile already tests everything). And in the future,
something like
{ dg-setenv "GOMP_MAPPING=prefer-copy" }
might get added to the test.
Tobias
