Re: [patch] OpenMP: Add -fopenmp-force-usm mode
On Wed, May 29, 2024 at 08:49:01AM +0200, Tobias Burnus wrote: > Jakub Jelinek wrote: > > I mean, if we want to add something, maybe better would an -include like > > option that instead of including a file includes it directly. > > gcc --include-inline '#pragma omp requires unified_shared_memory' ... > > Likewise for Fortran, but there the question is whether it should be in the > use-stmt, import-stmt, implicit-part or declaration-part; I guess having one > --include-inline-use-stmt and --include-inline-declaration would make sense Maybe name it slightly differently for Fortran and have the where it should be added as one argument, so --whatever=where=what > And, I guess, multiple flags should be permitted, which can then be > processed as separate lines. Obviously. That was the intent with --include-inline= for C as well, after all, -include works that way too. -include a.h -include b.h -include c.h Jakub
Re: [patch] OpenMP: Add -fopenmp-force-usm mode
On Wed, May 29, 2024 at 08:41:04AM +0200, Tobias Burnus wrote: > Jakub Jelinek wrote: > > How is that option different from > > echo '#pragma omp requires unified_shared_memory' > omp-usm.h > > gcc -include omp-usm.h > > ? > > I mean with -include you can add anything you want, not just one particular > > directive, and adding a separate option for each is just weird. > > For C/C++, -include seems to be indeed sufficient (albeit not widely known). > For Fortran, there at two issues: One placement/semantic issue: it has to be > added per "compilation unit", i.e. to the specification part of a module, > subprogram or main program. And a practical issue, gfortran shows: > > error: command-line option '-include !$omp requires' is valid for > C/C++/ObjC/ObjC++ but not for Fortran > > Thus, for Fortran it is still intrinsically useful – even if one can argue > whether that feature is needed at all / whether it should be added as > command-line argument. But then shouldn't we have an option that adds something at the start of the declaration part of each ? I mean, option to add 'implicit none' everywhere, or this '!$omp requires unified_shared_memory' etc.? I could live with an one off option for clang compatibility, I just fear that in 2 years we'll need another one etc. and that solving it in some more versatile way would be better. Jakub
Re: [patch] OpenMP: Add -fopenmp-force-usm mode
Jakub Jelinek wrote: I mean, if we want to add something, maybe better would an -include like option that instead of including a file includes it directly. gcc --include-inline '#pragma omp requires unified_shared_memory' ... Likewise for Fortran, but there the question is whether it should be in the use-stmt, import-stmt, implicit-part or declaration-part; I guess having one --include-inline-use-stmt and --include-inline-declaration would make sense … And, I guess, multiple flags should be permitted, which can then be processed as separate lines. Tobias
Re: [patch] OpenMP: Add -fopenmp-force-usm mode
Jakub Jelinek wrote: How is that option different from echo '#pragma omp requires unified_shared_memory' > omp-usm.h gcc -include omp-usm.h ? I mean with -include you can add anything you want, not just one particular directive, and adding a separate option for each is just weird. For C/C++, -include seems to be indeed sufficient (albeit not widely known). For Fortran, there at two issues: One placement/semantic issue: it has to be added per "compilation unit", i.e. to the specification part of a module, subprogram or main program. And a practical issue, gfortran shows: error: command-line option '-include !$omp requires' is valid for C/C++/ObjC/ObjC++ but not for Fortran Thus, for Fortran it is still intrinsically useful – even if one can argue whether that feature is needed at all / whether it should be added as command-line argument. Tobias
Re: [patch] OpenMP: Add -fopenmp-force-usm mode
On Wed, May 29, 2024 at 08:26:04AM +0200, Jakub Jelinek wrote: > > *I am especially thinking about a global variable and "#pragma omp declare > > target". At least with 'omp requires self_maps' of OpenMP 6, it seems as if > > 'declare target enter(global_var)' should become 'link(global_var)' where > > the global_var pointer is updated to point to the host version. > > How is that option different from > echo '#pragma omp requires unified_shared_memory' > omp-usm.h > gcc -include omp-usm.h > ? > I mean with -include you can add anything you want, not just one particular > directive, and adding a separate option for each is just weird. I mean, if we want to add something, maybe better would an -include like option that instead of including a file includes it directly. gcc --include-inline '#pragma omp requires unified_shared_memory' ... Jakub
Re: [patch] OpenMP: Add -fopenmp-force-usm mode
On Tue, May 28, 2024 at 09:23:41PM +0200, Tobias Burnus wrote: > -fopenmp-force-usm can be useful for some badly written code. Explicity > using 'omp requires' makes more sense but still. It might also make sense > for testing purpose. > > Unfortunately, I did not see a simple way of testing it. When trying it > manually, I looked at the 'a.xamdgcn-amdhsa.c' -save-temps file, where > gcn_data has the omp_requires_mask as second argument and testing showed > that an explicit pragma and the -f... argument have the same result. > > Alternative would be to move this code later, e.g. to lto-cgraph.cc's > omp_requires_mask, which might be safer (as it avoids changing as many > locations). On the other hand, it might require more special cases > elsewhere.* > > Comment, suggestions? > > Tobias > > *I am especially thinking about a global variable and "#pragma omp declare > target". At least with 'omp requires self_maps' of OpenMP 6, it seems as if > 'declare target enter(global_var)' should become 'link(global_var)' where > the global_var pointer is updated to point to the host version. How is that option different from echo '#pragma omp requires unified_shared_memory' > omp-usm.h gcc -include omp-usm.h ? I mean with -include you can add anything you want, not just one particular directive, and adding a separate option for each is just weird. Jakub
[patch] OpenMP: Add -fopenmp-force-usm mode
-fopenmp-force-usm can be useful for some badly written code. Explicity
using 'omp requires' makes more sense but still. It might also make
sense for testing purpose.
Unfortunately, I did not see a simple way of testing it. When trying it
manually, I looked at the 'a.xamdgcn-amdhsa.c' -save-temps file, where
gcn_data has the omp_requires_mask as second argument and testing showed
that an explicit pragma and the -f... argument have the same result.
Alternative would be to move this code later, e.g. to lto-cgraph.cc's
omp_requires_mask, which might be safer (as it avoids changing as many
locations). On the other hand, it might require more special cases
elsewhere.*
Comment, suggestions?
Tobias
*I am especially thinking about a global variable and "#pragma omp
declare target". At least with 'omp requires self_maps' of OpenMP 6, it
seems as if 'declare target enter(global_var)' should become
'link(global_var)' where the global_var pointer is updated to point to
the host version.
At least I don't see how otherwise the "all corresponding list items
created by the 'enter' clauses specified by declare target directives in
the compilation unit share storage with the original list items." could
be fulfilled.
This will require generating different code for 'self_maps' (and,
potentially / [RFC] 'unified_shared_memory') than normal code, which
would be the first compiler code-gen change due to USM (→
GOMP_OFFLOAD_CAP_SHARED_MEM) for non-host devices.
OpenMP: Add -fopenmp-force-usm mode
Add an implicit 'omp requires unified_shared_memory' to all files that
use target constructs ("OMP_REQUIRES_TARGET_USED"). As constructed, the
diagnostic "'unified_shared_memory' clause used lexically after first target
construct or offloading API" is not inhibited.
The option has no effect without -fopenmp and does not affect OpenACC code,
matching what the directive would do. The name of the command-line option
matches Clang's, added in LLVM 18.
gcc/c-family/ChangeLog:
* c.opt (fopenmp-force-usm): New.
* c.opt.urls: Regenerated
gcc/c/ChangeLog:
* c-parser.cc (c_parser_omp_target_data, c_parser_omp_target_update,
c_parser_omp_target_enter_data, c_parser_omp_target_exit_data,
c_parser_omp_target): When setting OMP_REQUIRES_TARGET_USED, also
set OMP_REQUIRES_UNIFIED_SHARED_MEMORY if -fopenmp-force-usm is
in force.
gcc/cp/ChangeLog:
* parser.cc (cp_parser_omp_target_data,
cp_parser_omp_target_enter_data, cp_parser_omp_target_exit_data,
cp_parser_omp_target_update, cp_parser_omp_target): When setting
OMP_REQUIRES_TARGET_USED, also set OMP_REQUIRES_UNIFIED_SHARED_MEMORY
if -fopenmp-force-usm is in force.
gcc/ChangeLog:
* doc/invoke.texi (-fopenmp-force-usm): Document new option.
gcc/fortran/ChangeLog:
* invoke.texi (-fopenmp-force-usm): Document new option.
* lang.opt (fopenmp-force-usm): New.
* lang.opt.urls: Regenerate.
* parse.cc (gfc_parse_file): When setting
OMP_REQUIRES_TARGET_USED, also set OMP_REQUIRES_UNIFIED_SHARED_MEMORY
if -fopenmp-force-usm is in force.
gcc/c-family/c.opt| 4
gcc/c-family/c.opt.urls | 3 +++
gcc/c/c-parser.cc | 50 +--
gcc/cp/parser.cc | 50 +--
gcc/doc/invoke.texi | 11 +--
gcc/fortran/invoke.texi | 7 +++
gcc/fortran/lang.opt | 4
gcc/fortran/lang.opt.urls | 3 +++
gcc/fortran/parse.cc | 10 --
9 files changed, 118 insertions(+), 24 deletions(-)
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index fb34c3b7031..4985cd61c48 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -2136,6 +2136,10 @@ fopenmp
C ObjC C++ ObjC++ LTO Var(flag_openmp)
Enable OpenMP (implies -frecursive in Fortran).
+fopenmp-force-usm
+C ObjC C++ ObjC++ Var(flag_openmp_force_usm)
+Behave as if the source file contained OpenMP's 'requires unified_shared_memory'.
+
fopenmp-simd
C ObjC C++ ObjC++ Var(flag_openmp_simd)
Enable OpenMP's SIMD directives.
diff --git a/gcc/c-family/c.opt.urls b/gcc/c-family/c.opt.urls
index dd455d7c0dc..34b3a395e84 100644
--- a/gcc/c-family/c.opt.urls
+++ b/gcc/c-family/c.opt.urls
@@ -1222,6 +1222,9 @@ UrlSuffix(gcc/C-Dialect-Options.html#index-fopenacc-dim)
fopenmp
UrlSuffix(gcc/C-Dialect-Options.html#index-fopenmp) LangUrlSuffix_Fortran(gfortran/Fortran-Dialect-Options.html#index-fopenmp)
+fopenmp-force-usm
+UrlSuffix(gcc/C-Dialect-Options.html#index-fopenmp-force-usm) LangUrlSuffix_Fortran(gfortran/Fortran-Dialect-Options.html#index-fopenmp-force-usm)
+
fopenmp-simd
UrlSuffix(gcc/C-Dialect-Options.html#index-fopenmp-simd) LangUrlSuffix_Fortran(gfortran/Fortran-Dialect-Options.html#index-fopenmp-simd)
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 00f8bf4376e..93c9cd1c9d0 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -23849,8 +23849,14 @@ static tree
c_parser_omp_target_data (location_t loc, c_parser *parser, bool *if_p)
{
if
