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
