Hi!
On 2021-08-19T22:13:56+0200, I wrote:
> On 2021-08-16T10:21:04+0200, Jakub Jelinek <ja...@redhat.com> wrote:
>> On Mon, Aug 16, 2021 at 10:08:42AM +0200, Thomas Schwinge wrote:
> |> Concerning the current 'gcc/omp-low.c:omp_build_component_ref', for the
> |> current set of offloading testcases, we never see a
> |> '!ADDR_SPACE_GENERIC_P' there, so the address space handling doesn't seem
> |> to be necessary there (but also won't do any harm: no-op).
>>
>> Are you sure this can't trigger?
>> Say
>> extern int __seg_fs a;
>>
>> void
>> foo (void)
>> {
>> #pragma omp parallel private (a)
>> a = 2;
>> }
>
> That test case doesn't run into 'omp_build_component_ref' at all,
> but [I've pushed an altered and extended variant that does],
> "Add 'libgomp.c/address-space-1.c'".
>
> In this case, 'omp_build_component_ref' called via host compilation
> 'pass_lower_omp', it's the 'field_type' that has 'address-space-1', not
> 'obj_type', so indeed Kwok's new code is a no-op:
>
> (gdb) call debug_tree(field_type)
> <pointer_type 0x7ffff7686b28
> type <integer_type 0x7ffff7686498 int address-space-1 SI
>> I think keeping the qual addr space here is the wrong thing to do,
>> it should keep the other quals and clear the address space instead,
>> the whole struct is going to be in generic addres space, isn't it?
>
> Correct for 'omp_build_component_ref' called via host compilation
> 'pass_lower_omp'
> However, regarding the former comment -- shouldn't we force generic
> address space for all 'tree' types read in via LTO streaming for
> offloading compilation? I assume that (in the general case) address
> spaces are never compatible between host and offloading compilation?
> For [...] "Add 'libgomp.c/address-space-1.c'", propagating the
> '__seg_fs' address space across the offloading boundary (assuming I did
> interpret the dumps correctly) doesn't seem to cause any problems
As I found later, actually the 'address-space-1' per host '__seg_fs' does
cause the "Intel MIC (emulated) offloading execution failure"
mentioned/XFAILed for 'libgomp.c/address-space-1.c': SIGSEGV, like
(expected) for host execution. For GCN offloading target, it maps to
GCN 'ADDR_SPACE_FLAT' which apparently doesn't cause any ill effects (for
that simple test case). The nvptx offloading target doesn't consider
address spaces at all.
Is the attached "Host and offload targets have no common meaning of
address spaces" OK to push?
Then, is that the way to do this, or should we add in
'gcc/tree-streamer-out.c:pack_ts_base_value_fields':
if (lto_stream_offload_p)
gcc_assert (ADDR_SPACE_GENERIC_P (TYPE_ADDR_SPACE (expr)));
..., and elsewhere sanitize this for offloading compilation? Jakub's
suggestion above, regarding 'gcc/omp-low.c:omp_build_component_ref':
| I think keeping the qual addr space here is the wrong thing to do,
| it should keep the other quals and clear the address space instead
But it's not obvious to me that indeed this is the one place where this
would need to be done? (It ought to work for
'libgomp.c/address-space-1.c', and any other occurrences would run into
the 'assert', so that ought to be "fine", though?)
And, should we have a new hook
'void targetm.addr_space.validate (addr_space_t as)' (better name?),
called via 'gcc/emit-rtl.c:set_mem_attrs' (only? -- assuming this is the
appropriate canonic function where address space use is observed?), to
make sure that the requested 'as' is valid for the target?
'default_addr_space_validate' would refuse everything but
'ADDR_SPACE_GENERIC_P (as)'; this hook would need implementing for all
handful of targets making use of address spaces (supposedly matching the
logic how they call 'c_register_addr_space'?). (The closest existing
hook seems to be 'targetm.addr_space.diagnose_usage', only defined for
AVR, and called from "the front ends" (C only).)
Grüße
Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From e01e06bd17bf2c7cb182d30bed02babc5edfa183 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Tue, 24 Aug 2021 11:14:10 +0200
Subject: [PATCH] Host and offload targets have no common meaning of address
spaces
gcc/
* tree-streamer-out.c (pack_ts_base_value_fields): Don't pack
'TYPE_ADDR_SPACE' for offloading.
* tree-streamer-in.c (unpack_ts_base_value_fields): Don't unpack
'TYPE_ADDR_SPACE' for offloading.
libgomp/
* testsuite/libgomp.c/address-space-1.c: Remove 'dg-xfail-run-if'
for 'offload_device_intel_mic'.
---
gcc/tree-streamer-in.c | 2 ++
gcc/tree-streamer-out.c | 4 +++-
libgomp/testsuite/libgomp.c/address-space-1.c | 4 ----
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/gcc/tree-streamer-in.c b/gcc/tree-streamer-in.c
index e0522bf2ac1..acdc48ef09f 100644
--- a/gcc/tree-streamer-in.c
+++ b/gcc/tree-streamer-in.c
@@ -146,7 +146,9 @@ unpack_ts_base_value_fields (struct bitpack_d *bp, tree expr)
TYPE_REVERSE_STORAGE_ORDER (expr) = (unsigned) bp_unpack_value (bp, 1);
else
TYPE_SATURATING (expr) = (unsigned) bp_unpack_value (bp, 1);
+#ifndef ACCEL_COMPILER
TYPE_ADDR_SPACE (expr) = (unsigned) bp_unpack_value (bp, 8);
+#endif
}
else if (TREE_CODE (expr) == BIT_FIELD_REF || TREE_CODE (expr) == MEM_REF)
{
diff --git a/gcc/tree-streamer-out.c b/gcc/tree-streamer-out.c
index 855d1cd59b9..aac0b7ecf54 100644
--- a/gcc/tree-streamer-out.c
+++ b/gcc/tree-streamer-out.c
@@ -119,7 +119,9 @@ pack_ts_base_value_fields (struct bitpack_d *bp, tree expr)
bp_pack_value (bp, TYPE_REVERSE_STORAGE_ORDER (expr), 1);
else
bp_pack_value (bp, TYPE_SATURATING (expr), 1);
- bp_pack_value (bp, TYPE_ADDR_SPACE (expr), 8);
+ /* Host and offload targets have no common meaning of address spaces. */
+ if (!lto_stream_offload_p)
+ bp_pack_value (bp, TYPE_ADDR_SPACE (expr), 8);
}
else if (TREE_CODE (expr) == BIT_FIELD_REF || TREE_CODE (expr) == MEM_REF)
{
diff --git a/libgomp/testsuite/libgomp.c/address-space-1.c b/libgomp/testsuite/libgomp.c/address-space-1.c
index 6ad57deec42..39ff82c1429 100644
--- a/libgomp/testsuite/libgomp.c/address-space-1.c
+++ b/libgomp/testsuite/libgomp.c/address-space-1.c
@@ -3,10 +3,6 @@
/* { dg-do run { target i?86-*-* x86_64-*-* } } */
/* { dg-require-effective-target offload_device_nonshared_as } */
-/* With Intel MIC (emulated) offloading:
- offload error: process on the device 0 unexpectedly exited with code 0
- { dg-xfail-run-if TODO { offload_device_intel_mic } } */
-
#include <assert.h>
int __seg_fs a;
--
2.25.1
