From: Thomas Schwinge <thomas@codesourcery.com>
To: Martin Sebor <msebor@gmail.com>, <gcc-patches@gcc.gnu.org>
Cc: Andrew Stubbs <ams@codesourcery.com>,
Hafiz Abid Qadeer <abidh@codesourcery.com>, <doko@debian.org>,
Jakub Jelinek <jakub@redhat.com>
Subject: Re: [gcn] Work-around libgomp 'error: array subscript 0 is outside array bounds of ‘__lds struct gomp_thread * __lds[0]’ [-Werror=array-bounds]'
Date: Tue, 20 Jul 2021 09:23:24 +0200 [thread overview]
Message-ID: <87v9558n4j.fsf@euler.schwinge.homeip.net> (raw)
In-Reply-To: <87bl6ybsic.fsf@euler.schwinge.homeip.net>
[-- Attachment #1: Type: text/plain, Size: 4466 bytes --]
Hi!
On 2021-07-19T10:46:35+0200, I wrote:
> | 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.
As I should find, these '#pragma GCC diagnostic [...]' directives cause
some code generation changes (that seems unexpected, problematic!).
(Martin, any idea? Might be a pre-existing problem, of course.) This
results in a lot (ten thousands) of 'GCN team arena exhausted' run-time
diagnostics, also leading to a few FAILs:
PASS: libgomp.c/../libgomp.c-c++-common/for-11.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-11.c execution test
PASS: libgomp.c/../libgomp.c-c++-common/for-12.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-12.c execution test
PASS: libgomp.c/../libgomp.c-c++-common/for-3.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-3.c execution test
PASS: libgomp.c/../libgomp.c-c++-common/for-5.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-5.c execution test
PASS: libgomp.c/../libgomp.c-c++-common/for-6.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-6.c execution test
PASS: libgomp.c/../libgomp.c-c++-common/for-9.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-9.c execution test
Same for 'libgomp.c++'.
It remains to be analyzed how '#pragma GCC diagnostic [...]' directives
can cause code generation changes; for now I'm working around the
"unexpected" '-Werror=array-bounds' diagnostics differently:
> |> (So I'll consider some GCN-specific '-Wno-array-bounds' if we don't
> |> get to resolve this otherwise, soon.)
'-Wno-error=array-bounds', precisely. I've now pushed "[gcn]
Work-around libgomp 'error: array subscript 0 is outside array bounds of
‘__lds struct gomp_thread * __lds[0]’ [-Werror=array-bounds]' some more
[PR101484]" to master branch in commit
8168338684fc2bed576bb09202c63b3e9e678d92, see attached.
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
[-- Attachment #2: 0001-gcn-Work-around-libgomp-error-array-subscript-0-is-o.patch --]
[-- Type: text/x-diff, Size: 5890 bytes --]
From 8168338684fc2bed576bb09202c63b3e9e678d92 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Mon, 19 Jul 2021 23:11:38 +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]'=20some=20more=20[PR101484?=
=?UTF-8?q?]?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
With yesterday's commit 9f2bc5077debef2b046b6c10d38591ac324ad8b5 "[gcn]
Work-around libgomp 'error: array subscript 0 is outside array bounds of
‘__lds struct gomp_thread * __lds[0]’ [-Werror=array-bounds]' [PR101484]",
I did defuse the "unexpected" '-Werror=array-bounds' diagnostics that we see
as of commit a110855667782dac7b674d3e328b253b3b3c919b "Correct handling of
variable offset minus constant in -Warray-bounds [PR100137]". However, these
'#pragma GCC diagnostic [...]' directives cause some code generation changes
(that seems unexpected, problematic!), which results in a lot (ten thousands)
of 'GCN team arena exhausted' run-time diagnostics, also leading to a few
FAILs:
PASS: libgomp.c/../libgomp.c-c++-common/for-11.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-11.c execution test
PASS: libgomp.c/../libgomp.c-c++-common/for-12.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-12.c execution test
PASS: libgomp.c/../libgomp.c-c++-common/for-3.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-3.c execution test
PASS: libgomp.c/../libgomp.c-c++-common/for-5.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-5.c execution test
PASS: libgomp.c/../libgomp.c-c++-common/for-6.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-6.c execution test
PASS: libgomp.c/../libgomp.c-c++-common/for-9.c (test for excess errors)
[-PASS:-]{+FAIL:+} libgomp.c/../libgomp.c-c++-common/for-9.c execution test
Same for 'libgomp.c++'.
It remains to be analyzed how '#pragma GCC diagnostic [...]' directives can
cause code generation changes; for now I'm working around the "unexpected"
'-Werror=array-bounds' diagnostics differently.
Overall, still awaiting a different solution, of course.
libgomp/
PR target/101484
* configure.tgt [amdgcn*-*-*] (XCFLAGS): Add
'-Wno-error=array-bounds'.
* config/gcn/team.c: Remove '-Werror=array-bounds' work-around.
* libgomp.h [__AMDGCN__]: Likewise.
---
libgomp/config/gcn/team.c | 3 ---
libgomp/configure.tgt | 3 +++
libgomp/libgomp.h | 12 ------------
3 files changed, 3 insertions(+), 15 deletions(-)
diff --git a/libgomp/config/gcn/team.c b/libgomp/config/gcn/team.c
index 94ce2f2dfeb..627210ea407 100644
--- a/libgomp/config/gcn/team.c
+++ b/libgomp/config/gcn/team.c
@@ -65,12 +65,9 @@ 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/configure.tgt b/libgomp/configure.tgt
index fe2bf1dac51..d4f1e741b5a 100644
--- a/libgomp/configure.tgt
+++ b/libgomp/configure.tgt
@@ -173,6 +173,9 @@ case "${target}" in
amdgcn*-*-*)
config_path="gcn accel"
+
+ #TODO PR101484
+ XCFLAGS="$XCFLAGS -Wno-error=array-bounds"
;;
*)
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 4159cbe3334..8d25dc8e2a8 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -128,10 +128,7 @@ 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. */
@@ -162,11 +159,8 @@ 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
@@ -795,19 +789,13 @@ 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
next prev parent reply other threads:[~2021-07-20 7:23 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-09 23:11 [PATCH libatomic/arm] avoid warning on constant addresses (PR 101379) Martin Sebor
2021-07-15 8:33 ` Christophe Lyon
2021-07-16 17:42 ` Thomas Schwinge
2021-07-16 21:11 ` Martin Sebor
2021-07-19 8:49 ` Thomas Schwinge
2021-07-17 22:28 ` Andrew Stubbs
2021-07-19 8:46 ` [gcn] Work-around libgomp 'error: array subscript 0 is outside array bounds of ‘__lds struct gomp_thread * __lds[0]’ [-Werror=array-bounds]' (was: [PATCH libatomic/arm] avoid warning on constant addresses (PR 101379)) Thomas Schwinge
2021-07-19 8:56 ` Jakub Jelinek
2021-07-19 11:10 ` Andrew Stubbs
2021-07-20 7:23 ` Thomas Schwinge [this message]
2021-07-20 8:40 ` '#pragma GCC diagnostic' (mis-)use in 'statement' of 'if' (was: [gcn] Work-around libgomp 'error: array subscript 0 is outside array bounds of ‘__lds struct gomp_thread * __lds[0]’ [-Werror=array-bounds]') Thomas Schwinge
2021-07-20 19:47 ` '#pragma GCC diagnostic' (mis-)use in 'statement' of 'if' Martin Sebor
2021-07-20 20:16 ` Jakub Jelinek
2021-07-21 16:41 ` [PATCH libatomic/arm] avoid warning on constant addresses (PR 101379) Kyrylo Tkachov
2021-07-21 16:54 ` Martin Sebor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87v9558n4j.fsf@euler.schwinge.homeip.net \
--to=thomas@codesourcery.com \
--cc=abidh@codesourcery.com \
--cc=ams@codesourcery.com \
--cc=doko@debian.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=msebor@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).