| Message ID | 87bl6ybsic.fsf@euler.schwinge.homeip.net |
|---|---|
| State | New |
| Headers | show |
| Series | [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)) | expand |
On Mon, Jul 19, 2021 at 10:46:35AM +0200, Thomas Schwinge wrote: > |> (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." ;-) The current state of the warning is unacceptable. IMNSHO it should stop warning for non-generic address spaces and for generic address space for constants larger or equal to typical page size with -fdelete-null-pointer-checks and not at all with -fno-delete-null-pointer-checks. Large structures are rare and using *(type *)0x12340000 is very common mostly in the embedded world, but with mmap MAP_FIXED also used in non-embedded world. We don't want all the people out there to add too many workarounds for badly designed warning. Jakub
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. Andrew
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
Hi! On 2021-07-20T09:23:24+0200, I wrote: > 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.) OK, phew. Martin: your diagnostic changes are *not* to be blamed for code generation changes -- it's my '#pragma GCC diagnostic pop' placement that triggers: > 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: In addition to a few in straight-line code, I also had these two: > --- 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. */ > @@ -162,8 +159,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 ..., and it appears that the '#pragma GCC diagnostic pop' are considered here to be the 'statement' of the 'if'! That's (a) unexpected (to me, at least) for this kind of "non-executable" '#pragma', and (b) certainly would be worth a dignostic, like we have for OMP pragmas, for example: if (context == pragma_stmt) { error_at (loc, "%<#pragma %s%> may only be used in compound statements", "[...]"); Addressing that is for another day. 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
On 7/20/21 2:40 AM, Thomas Schwinge wrote: > Hi! > > On 2021-07-20T09:23:24+0200, I wrote: >> 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.) > > OK, phew. Martin: your diagnostic changes are *not* to be blamed for > code generation changes -- it's my '#pragma GCC diagnostic pop' > placement that triggers: > >> 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: > > In addition to a few in straight-line code, I also had these two: > >> --- 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. */ >> @@ -162,8 +159,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 > > ..., and it appears that the '#pragma GCC diagnostic pop' are considered > here to be the 'statement' of the 'if'! That's (a) unexpected (to me, at > least) for this kind of "non-executable" '#pragma', and (b) certainly > would be worth a dignostic, like we have for OMP pragmas, for example: > > if (context == pragma_stmt) > { > error_at (loc, "%<#pragma %s%> may only be used in compound statements", > "[...]"); > I agree, that does seem quite surprising. I opened pr101538 only to find that the problem is already tracked in pr63326. > Addressing that is for another day. David Malcolm (CC'd) has a patch attached to pr63326 to issue a warning to point out that #pragmas are treated as statements that would help prevent this type of a bug. David, do you still plan to submit it? Martin
On Tue, Jul 20, 2021 at 01:47:01PM -0600, Martin Sebor wrote: > > Addressing that is for another day. > > David Malcolm (CC'd) has a patch attached to pr63326 to issue > a warning to point out that #pragmas are treated as statements > that would help prevent this type of a bug. David, do you still > plan to submit it? That patch doesn't look correct. c_parser_pragma and cp_parser_pragma is already told if it appears in a context for which treating the pragma as standalone statement changes the behavior and in contexts where it doesn't - pragma_stmt stands for the problematic ones, pragma_compound for the correct ones (there are other values for namespace scope, class scope etc.). OpenMP/OpenACC pragmas shouldn't be touched, those already do the right thing the standard asks for, for the remaining ones there should be a warning for the pragma_stmt cases. Jakub
From 9f2bc5077debef2b046b6c10d38591ac324ad8b5 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge <thomas@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
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 <ams@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