[gcn,nvptx] Add warning to mkoffload for 32bit host code
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
Motivated by a surprise of a colleague that with -m32,
no offload dumps were created; that's because mkoffload
does not process host binaries when the are 32bit (i.e. ilp32).
Internally, that done as follows: The host compiler passes to
'mkoffload' the used host ABI, i.e. -foffload-abi=ilp32 or -foffload-abi=lp64
That's done via TARGET_OFFLOAD_OPTIONS, which is supported by aarch64, i386, and rs6000.
While it is sensible (albeit not strictly required) that GCC requires that
the host and device side agree and that only 64bit is implemented for the
device side, it can be confusing that silently no offloading code is generated.
Hence, I propose to print a warning in that case - as implemented in the attached patch:
$ gcc -fopenmp -m32 test.c
nvptx mkoffload: warning: offload code generation skipped: offloading with 32-bit host code is currently not supported
gcn mkoffload: warning: offload code generation skipped: offloading with 32-bit host code is currently not supported
* * *
This shouldn't have any effect on offload builds using -m64
and non-offload builds – while several testcases already have
issues with '-m32' when offloading is enabled or an offloading
device is available.
To make it not worse, this patch adds some pruning and for
a subset of the failing testcases, I added code to avoids FAILS.
There are some more fails, but those aren't new.
Comments, remarks, suggestions?
Is the mkoffload.cc part is okay?
Tobias
Comments
On 25/04/2024 11:51, Tobias Burnus wrote:
> Motivated by a surprise of a colleague that with -m32,
> no offload dumps were created; that's because mkoffload
> does not process host binaries when the are 32bit (i.e. ilp32).
>
> Internally, that done as follows: The host compiler passes to
> 'mkoffload' the used host ABI, i.e. -foffload-abi=ilp32 or -foffload-abi=lp64
>
> That's done via TARGET_OFFLOAD_OPTIONS, which is supported by aarch64, i386, and rs6000.
>
> While it is sensible (albeit not strictly required) that GCC requires that
> the host and device side agree and that only 64bit is implemented for the
> device side, it can be confusing that silently no offloading code is generated.
>
>
> Hence, I propose to print a warning in that case - as implemented in the attached patch:
>
> $ gcc -fopenmp -m32 test.c
> nvptx mkoffload: warning: offload code generation skipped: offloading with 32-bit host code is currently not supported
> gcn mkoffload: warning: offload code generation skipped: offloading with 32-bit host code is currently not supported
>
> * * *
>
> This shouldn't have any effect on offload builds using -m64
> and non-offload builds – while several testcases already have
> issues with '-m32' when offloading is enabled or an offloading
> device is available.
>
> To make it not worse, this patch adds some pruning and for
> a subset of the failing testcases, I added code to avoids FAILS.
> There are some more fails, but those aren't new.
>
> Comments, remarks, suggestions?
> Is the mkoffload.cc part is okay?
The mkoffload part looks reasonable to me. I'm not sure if there are
other ABIs we might want to warn about, but this is definitely an
improvement.
Andrew
[gcn][nvptx] Add warning to mkoffload for 32bit host code
mkoffload in principle handles 32bit and 64bit offload targets,
but 32bit support has no been implemented. Before this patch,
offloading is then silently disabled for the respective target.
With the patch, the user gets a warning by mkoffload (and the
programm continues to be build with out offloading code).
gcc/ChangeLog:
* config/gcn/mkoffload.cc (main): Warn for -foffload-abi=ilp32
that no offload code will be generated.
* config/nvptx/mkoffload.cc (main): Likewise.
libgomp/ChangeLog:
* testsuite/lib/libgomp-dg.exp (libgomp-dg-prune): Prune warning
by mkoffload that 32-bit offloading is not supported.
* testsuite/libgomp.c-c++-common/requires-1.c: Silence a FAIL for
'ia32' targets as for them no offload code is generated.
* testsuite/libgomp.c-c++-common/requires-3.c: Likewise.
* testsuite/libgomp.c-c++-common/requires-7.c: Likewise.
* testsuite/libgomp.c-c++-common/variable-not-offloaded.c: Likewise.
* testsuite/libgomp.fortran/requires-1.f90: Likewise.
gcc/config/gcn/mkoffload.cc | 5 ++++-
gcc/config/nvptx/mkoffload.cc | 5 ++++-
libgomp/testsuite/lib/libgomp-dg.exp | 3 +++
libgomp/testsuite/libgomp.c-c++-common/requires-1.c | 8 +++++---
libgomp/testsuite/libgomp.c-c++-common/requires-3.c | 8 +++++---
libgomp/testsuite/libgomp.c-c++-common/requires-7.c | 10 ++++++----
.../testsuite/libgomp.c-c++-common/variable-not-offloaded.c | 4 ++--
libgomp/testsuite/libgomp.fortran/requires-1.f90 | 8 +++++---
8 files changed, 34 insertions(+), 17 deletions(-)
@@ -1143,7 +1143,10 @@ main (int argc, char **argv)
fatal_error (input_location, "cannot open %qs", gcn_cfile_name);
/* Currently, we only support offloading in 64-bit configurations. */
- if (offload_abi == OFFLOAD_ABI_LP64)
+ if (offload_abi == OFFLOAD_ABI_ILP32)
+ warning (0, "offload code generation skipped: offloading with 32-bit host "
+ "code is currently not supported");
+ else if (offload_abi == OFFLOAD_ABI_LP64)
{
const char *mko_dumpbase = concat (dumppfx, ".mkoffload", NULL);
const char *hsaco_dumpbase = concat (dumppfx, ".mkoffload.hsaco", NULL);
@@ -798,7 +798,10 @@ main (int argc, char **argv)
/* PR libgomp/65099: Currently, we only support offloading in 64-bit
configurations. */
- if (offload_abi == OFFLOAD_ABI_LP64)
+ if (offload_abi == OFFLOAD_ABI_ILP32)
+ warning (0, "offload code generation skipped: offloading with 32-bit host "
+ "code is currently not supported");
+ else if (offload_abi == OFFLOAD_ABI_LP64)
{
char *mko_dumpbase = concat (dumppfx, ".mkoffload", NULL);
if (save_temps)
@@ -3,5 +3,8 @@ proc libgomp-dg-test { prog do_what extra_tool_flags } {
}
proc libgomp-dg-prune { system text } {
+ global additional_prunes
+ lappend additional_prunes "offloading with 32-bit host code is currently not supported"
+
return [gcc-dg-prune $system $text]
}
@@ -20,7 +20,9 @@ main (void)
return 0;
}
-/* { dg-error "OpenMP 'requires' directive with non-identical clauses in multiple compilation units: 'unified_shared_memory' vs. 'unified_address'" "" { target *-*-* } 0 }
- { dg-note {requires-1\.c' has 'unified_shared_memory'} {} { target *-*-* } 0 }
- { dg-note {requires-1-aux\.c' has 'unified_address'} {} { target *-*-* } 0 } */
+/* Skip for ia32 as mkoffload requires 64 bit (and warns otherwise); cf. PR libgomp/65099. */
+
+/* { dg-error "OpenMP 'requires' directive with non-identical clauses in multiple compilation units: 'unified_shared_memory' vs. 'unified_address'" "" { target { ! ia32 } } 0 }
+ { dg-note {requires-1\.c' has 'unified_shared_memory'} {} { target { ! ia32 } } 0 }
+ { dg-note {requires-1-aux\.c' has 'unified_address'} {} { target { ! ia32 } } 0 } */
/* { dg-excess-errors "Ignore messages like: errors during merging of translation units|mkoffload returned 1 exit status" } */
@@ -20,7 +20,9 @@ main (void)
return 0;
}
-/* { dg-error "OpenMP 'requires' directive with non-identical clauses in multiple compilation units: 'unified_address, unified_shared_memory' vs. 'unified_address'" "" { target *-*-* } 0 }
- { dg-note {requires-3\.c' has 'unified_address, unified_shared_memory'} {} { target *-*-* } 0 }
- { dg-note {requires-3-aux\.c' has 'unified_address'} {} { target *-*-* } 0 } */
+/* Skip for ia32 as mkoffload requires 64 bit (and warns otherwise); cf. PR libgomp/65099. */
+
+/* { dg-error "OpenMP 'requires' directive with non-identical clauses in multiple compilation units: 'unified_address, unified_shared_memory' vs. 'unified_address'" "" { target { ! ia32 } } 0 }
+ { dg-note {requires-3\.c' has 'unified_address, unified_shared_memory'} {} { target { ! ia32 } } 0 }
+ { dg-note {requires-3-aux\.c' has 'unified_address'} {} { target { ! ia32 } } 0 } */
/* { dg-excess-errors "Ignore messages like: errors during merging of translation units|mkoffload returned 1 exit status" } */
@@ -20,13 +20,15 @@ main (void)
return 0;
}
-/* { dg-error "OpenMP 'requires' directive with non-identical clauses in multiple compilation units: 'unified_shared_memory' vs. 'unified_address'" "" { target *-*-* } 0 }
- { dg-note {requires-7\.c' has 'unified_shared_memory'} {} { target *-*-* } 0 }
+/* Skip for ia32 as mkoffload requires 64 bit (and warns otherwise); cf. PR libgomp/65099. */
+
+/* { dg-error "OpenMP 'requires' directive with non-identical clauses in multiple compilation units: 'unified_shared_memory' vs. 'unified_address'" "" { target { ! ia32 } } 0 }
+ { dg-note {requires-7\.c' has 'unified_shared_memory'} {} { target { ! ia32 } } 0 }
TODO We're currently not streaming location information for the OpenMP
directives used in 'requires-7-aux.c', so we're not seeing the source file
name here (but a temporary '*.o' instead; for details, see
<https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598011.html>):
- { dg-note {requires-7-aux\.c' has 'unified_address'} {} { xfail *-*-* } 0 }
+ { dg-note {requires-7-aux\.c' has 'unified_address'} {} { xfail { *-*-* } } 0 }
..., but we may still verify that the rest of the diagnostic is correct:
- { dg-note {' has 'unified_address'} {} { target *-*-* } 0 } */
+ { dg-note {' has 'unified_address'} {} { target { ! ia32 } } 0 } */
/* { dg-excess-errors "Ignore messages like: errors during merging of translation units|mkoffload returned 1 exit status" } */
@@ -1,7 +1,7 @@
/* { dg-do link } */
-/* { dg-excess-errors "lto1, mkoffload and lto-wrapper fatal errors" { target { offload_target_nvptx || offload_target_amdgcn } } } */
+/* { dg-excess-errors "lto1, mkoffload and lto-wrapper fatal errors" { target { { ! ia32 } && { offload_target_nvptx || offload_target_amdgcn } } } } */
-int var; /* { dg-error "variable 'var' has been referenced in offloaded code but hasn't been marked to be included in the offloaded code" "" { target { offload_target_nvptx || offload_target_amdgcn } } } */
+int var; /* { dg-error "variable 'var' has been referenced in offloaded code but hasn't been marked to be included in the offloaded code" "" { target { { ! ia32 } && { offload_target_nvptx || offload_target_amdgcn } } } } */
#pragma omp declare target
void __attribute__((noinline, noclone))
@@ -22,13 +22,15 @@ program main
call foo ()
end
-! { dg-error "OpenMP 'requires' directive with non-identical clauses in multiple compilation units: 'unified_shared_memory' vs. 'unified_address'" "" { target *-*-* } 0 }
-! { dg-note {requires-1\.f90' has 'unified_shared_memory'} {} { target *-*-* } 0 }
+! Skip for ia32 as mkoffload requires 64 bit (and warns otherwise); cf. PR libgomp/65099.
+
+! { dg-error "OpenMP 'requires' directive with non-identical clauses in multiple compilation units: 'unified_shared_memory' vs. 'unified_address'" "" { target { ! ia32 } } 0 }
+! { dg-note {requires-1\.f90' has 'unified_shared_memory'} {} { target { ! ia32 } } 0 }
! TODO We're currently not streaming location information for the OpenMP
! directives used in 'requires-7-aux.c', so we're not seeing the source file
! name here (but a temporary '*.o' instead; for details, see
! <https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598011.html>):
! { dg-note {requires-1-aux\.f90' has 'unified_address'} {} { xfail *-*-* } 0 }
! ..., but we may still verify that the rest of the diagnostic is correct:
-! { dg-note {' has 'unified_address'} {} { target *-*-* } 0 }
+! { dg-note {' has 'unified_address'} {} { target { ! ia32 } } 0 }
! { dg-excess-errors "Ignore messages like: errors during merging of translation units|mkoffload returned 1 exit status" }