Hi!

| On 7/16/21 11:42 AM, Thomas Schwinge wrote:
|> On 2021-07-09T17:11:25-0600, Martin Sebor via Gcc-patches 
<gcc-patches@gcc.gnu.org> wrote:
|>> The attached tweak avoids the new -Warray-bounds instances when
|>> building libatomic for arm. Christophe confirms it resolves
|>> the problem (thank you!)
|>
|> As Abid has just reported in
|> <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.  Work-around:
|>
|>         struct gomp_thread * __lds *thrs = (struct gomp_thread * __lds *)4;
|>      +# pragma GCC diagnostic push
|>      +# pragma GCC diagnostic ignored "-Warray-bounds"
|>         return *thrs;
|>      +# pragma GCC diagnostic pop
|>
|> ..., but it's a bit tedious to add that in all that the other places,
|> too.

Wasn't so bad after all; a lot of duplicates due to 'libgomp.h'.  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]" to master branch in commit
9f2bc5077debef2b046b6c10d38591ac324ad8b5, see attached.

|> (So I'll consider some GCN-specific '-Wno-array-bounds' if we don't
|> get to resolve this otherwise, soon.)

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


On 2021-07-17T23:28:45+0100, Andrew Stubbs <a...@codesourcery.com> wrote:
> On 16/07/2021 18:42, Thomas Schwinge wrote:
>> Of course, we may simply re-work the libgomp/GCN code -- but don't we
>> first need to answer the question whether the current code is actually
>> "bad"?  Aren't we going to get a lot of similar reports from
>> kernel/embedded/other low-level software developers, once this is out in
>> the wild?  I mean:
>
> 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 really ought not be this hard. :-(

I'm sure we can avoid that.


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 9f2bc5077debef2b046b6c10d38591ac324ad8b5 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Fri, 16 Jul 2021 19:12:02 +0200
Subject: [PATCH] =?UTF-8?q?[gcn]=20Work-around=20libgomp=20'error:=20array?=
 =?UTF-8?q?=20subscript=200=20is=20outside=20array=20bounds=20of=20?=
 =?UTF-8?q?=E2=80=98=5F=5Flds=20struct=20gomp=5Fthread=20*=20=5F=5Flds[0]?=
 =?UTF-8?q?=E2=80=99=20[-Werror=3Darray-bounds]'=20[PR101484]?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

... seen as of commit a110855667782dac7b674d3e328b253b3b3c919b "Correct
handling of variable offset minus constant in -Warray-bounds [PR100137]".

Awaiting a different solution, of course.

	libgomp/
	PR target/101484
	* config/gcn/team.c: Apply '-Werror=array-bounds' work-around.
	* libgomp.h [__AMDGCN__]: Likewise.
---
 libgomp/config/gcn/team.c |  3 +++
 libgomp/libgomp.h         | 12 ++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/libgomp/config/gcn/team.c b/libgomp/config/gcn/team.c
index 627210ea407..94ce2f2dfeb 100644
--- a/libgomp/config/gcn/team.c
+++ b/libgomp/config/gcn/team.c
@@ -65,9 +65,12 @@ gomp_gcn_enter_kernel (void)
       void * __lds *arena_start = (void * __lds *)TEAM_ARENA_START;
       void * __lds *arena_free = (void * __lds *)TEAM_ARENA_FREE;
       void * __lds *arena_end = (void * __lds *)TEAM_ARENA_END;
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Warray-bounds" /*TODO PR101484 */
       *arena_start = team_arena;
       *arena_free = team_arena;
       *arena_end = team_arena + TEAM_ARENA_SIZE;
+# pragma GCC diagnostic pop
 
       /* Allocate and initialize the team-local-storage data.  */
       struct gomp_thread *thrs = team_malloc_cleared (sizeof (*thrs)
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 8d25dc8e2a8..4159cbe3334 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -128,7 +128,10 @@ team_malloc (size_t size)
        : "=v"(result) : "v"(TEAM_ARENA_FREE), "v"(size), "e"(1L) : "memory");
 
   /* Handle OOM.  */
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Warray-bounds" /*TODO PR101484 */
   if (result + size > *(void * __lds *)TEAM_ARENA_END)
+# pragma GCC diagnostic pop
     {
       /* While this is experimental, let's make sure we know when OOM
 	 happens.  */
@@ -159,8 +162,11 @@ team_free (void *ptr)
      However, if we fell back to using heap then we should free it.
      It would be better if this function could be a no-op, but at least
      LDS loads are cheap.  */
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Warray-bounds" /*TODO PR101484 */
   if (ptr < *(void * __lds *)TEAM_ARENA_START
       || ptr >= *(void * __lds *)TEAM_ARENA_END)
+# pragma GCC diagnostic pop
     free (ptr);
 }
 #else
@@ -789,13 +795,19 @@ static inline struct gomp_thread *gcn_thrs (void)
 {
   /* The value is at the bottom of LDS.  */
   struct gomp_thread * __lds *thrs = (struct gomp_thread * __lds *)4;
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Warray-bounds" /*TODO PR101484 */
   return *thrs;
+# pragma GCC diagnostic pop
 }
 static inline void set_gcn_thrs (struct gomp_thread *val)
 {
   /* The value is at the bottom of LDS.  */
   struct gomp_thread * __lds *thrs = (struct gomp_thread * __lds *)4;
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Warray-bounds" /*TODO PR101484 */
   *thrs = val;
+# pragma GCC diagnostic pop
 }
 static inline struct gomp_thread *gomp_thread (void)
 {
-- 
2.30.2

Reply via email to