On 12/06/2026 15:08, Thomas Schwinge wrote:
Hi!

On 2021-07-19T10:46:35+0200, I wrote:
| On 7/16/21 11:42 AM, Thomas Schwinge wrote:
|> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101374#c16>, similar
|> problem with GCN target libgomp build:
|>
|>      In function ‘gcn_thrs’,
|>          inlined from ‘gomp_thread’ at 
[...]/source-gcc/libgomp/libgomp.h:803:10,
|>          inlined from ‘GOMP_barrier’ at 
[...]/source-gcc/libgomp/barrier.c:34:29:
|>      [...]/source-gcc/libgomp/libgomp.h:792:10: error: array subscript 0 is 
outside array bounds of ‘__lds struct gomp_thread * __lds[0]’ 
[-Werror=array-bounds]
|>        792 |   return *thrs;
|>            |          ^~~~~
|>
|>      gcc/config/gcn/gcn.h:  c_register_addr_space ("__lds", ADDR_SPACE_LDS); 
                  \
|>
|>      libgomp/libgomp.h-static inline struct gomp_thread *gcn_thrs (void)
|>      libgomp/libgomp.h-{
|>      libgomp/libgomp.h-  /* The value is at the bottom of LDS.  */
|>      libgomp/libgomp.h:  struct gomp_thread * __lds *thrs = (struct 
gomp_thread * __lds *)4;
|>      libgomp/libgomp.h-  return *thrs;
|>      libgomp/libgomp.h-}
|>
|> ..., plus a few more.  [...]

[...] I've
thus pushed "[gcn] Work-around libgomp 'error: array subscript 0 is
outside array bounds of ‘__lds struct gomp_thread * __lds[0]’
[-Werror=array-bounds]' [PR101484]" [...]

Now: "Awaiting a different solution, of course."  ;-)

Well, I just stumbled over that one; please have a look at the attached
"GCN: Implement 'TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID' [PR101484]".

I'm not very familiar with the GCN named address spaces -- in addition to
'__lds', which ones to include in 'TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID',
and/or instead of 'as == ADDR_SPACE_LDS', which 'AS_[...]_P' conditional
to use?

I did not know about this hook (or is it new?), so I can't comment on that usage, but the rest of the patch looks good.

I think only LDS has this problem*, and technically you could have used AS_LDS_P, but that only exists for parity with the more complicated cases so you're good.

OK.

Andrew



* GDS might too, but we don't use that, I suspect the support that is there is broken, and some devices might not have it.


'gcc/config/gcn/gcn.h':

     /* Address spaces.  */
     enum gcn_address_spaces
     {
       ADDR_SPACE_DEFAULT = 0,
       ADDR_SPACE_FLAT,
       ADDR_SPACE_SCALAR_FLAT,
       ADDR_SPACE_FLAT_SCRATCH,
       ADDR_SPACE_LDS,
       ADDR_SPACE_GDS,
       ADDR_SPACE_SCRATCH,
       ADDR_SPACE_GLOBAL
     };
     #define REGISTER_TARGET_PRAGMAS() do {                               \
       c_register_addr_space ("__flat", ADDR_SPACE_FLAT);                 \
       c_register_addr_space ("__flat_scratch", ADDR_SPACE_FLAT_SCRATCH); \
       c_register_addr_space ("__scalar_flat", ADDR_SPACE_SCALAR_FLAT);   \
       c_register_addr_space ("__lds", ADDR_SPACE_LDS);                   \
       c_register_addr_space ("__gds", ADDR_SPACE_GDS);                   \
       c_register_addr_space ("__global", ADDR_SPACE_GLOBAL);             \
     } while (0);
#define STACK_ADDR_SPACE ADDR_SPACE_GLOBAL
     #define DEFAULT_ADDR_SPACE \
       ((cfun && cfun->machine && !cfun->machine->use_flat_addressing) \
        ? ADDR_SPACE_GLOBAL : ADDR_SPACE_FLAT)
     #define AS_SCALAR_FLAT_P(AS)   ((AS) == ADDR_SPACE_SCALAR_FLAT)
     #define AS_FLAT_SCRATCH_P(AS)  ((AS) == ADDR_SPACE_FLAT_SCRATCH)
     #define AS_FLAT_P(AS)             ((AS) == ADDR_SPACE_FLAT \
                                || ((AS) == ADDR_SPACE_DEFAULT \
                                    && DEFAULT_ADDR_SPACE == ADDR_SPACE_FLAT))
     #define AS_LDS_P(AS)              ((AS) == ADDR_SPACE_LDS)
     #define AS_GDS_P(AS)              ((AS) == ADDR_SPACE_GDS)
     #define AS_SCRATCH_P(AS)       ((AS) == ADDR_SPACE_SCRATCH)
     #define AS_GLOBAL_P(AS)        ((AS) == ADDR_SPACE_GLOBAL \
                                || ((AS) == ADDR_SPACE_DEFAULT \
                                    && DEFAULT_ADDR_SPACE == ADDR_SPACE_GLOBAL))
     #define AS_ANY_FLAT_P(AS)      (AS_FLAT_SCRATCH_P (AS) || AS_FLAT_P (AS))
     #define AS_ANY_DS_P(AS)           (AS_LDS_P (AS) || AS_GDS_P (AS))


Additionally:

On 2021-07-19T12:10:59+0100, Andrew Stubbs <[email protected]> wrote:
On 19/07/2021 09:46, Thomas Schwinge wrote:
GCN already uses address 4 for this value because address 0 caused
problems with null-pointer checks.

Ugh.  How much wasted bytes per what is that?  (I haven't looked yet;
hopefully not per GPU thread?)  Because:

It's 4 bytes per gang. And that pointer is the only 8 bytes in the whole
of LDS (OpenMP mostly uses stack and heap), so it's not so bad, but still.

I did investigate the target macro that lets you control null pointer
behaviour, but it didn't just work, and it wasn't important enough for
me to spend more time on it so I let it go.

Maybe what we get per the above ('TARGET_ADDR_SPACE_ZERO_ADDRESS_VALID')
is already sufficient to also get rid of having to use "address 4 for
this value because address 0 caused problems with null-pointer checks"?
(I've not yet tried.)

If it does what it says then yes, we probably can remove that padding.

Andrew

Reply via email to