On Tue, May 3, 2022 at 2:29 PM Thomas Schwinge <tho...@codesourcery.com> wrote:
>
> Hi Richard!
>
> On 2022-05-03T12:53:50+0200, Richard Biener <richard.guent...@gmail.com>
> wrote:
> > On Tue, May 3, 2022 at 10:16 AM Thomas Schwinge <tho...@codesourcery.com>
> > wrote:
> >> On 2022-05-03T09:17:52+0200, Richard Biener <richard.guent...@gmail.com>
> >> wrote:
> >> > On Mon, May 2, 2022 at 4:01 PM Thomas Schwinge <tho...@codesourcery.com>
> >> > wrote:
> >> >> On 2022-05-01T11:02:29+0100, Iain Sandoe via Gcc <g...@gcc.gnu.org>
> >> >> wrote:
> >> >> >> On 29 Apr 2022, at 15:34, Jakub Jelinek via Gcc <g...@gcc.gnu.org>
> >> >> >> wrote:
> >> >> >>
> >> >> >> The first release candidate for GCC 12.1 is available from
> >> >> >>
> >> >> >> https://gcc.gnu.org/pub/gcc/snapshots/12.1.0-RC-20220429/
> >> >> >> ftp://gcc.gnu.org/pub/gcc/snapshots/12.1.0-RC-20220429/
> >> >> >>
> >> >> >> and shortly its mirrors. It has been generated from git commit
> >> >> >> r12-8321-g621650f64fb667.
> >> >>
> >> >> > [...] new fails (presumably because checking is off):
> >> >>
> >> >> > XPASS: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c
> >> >> > -std=c++98 (internal compiler error)
> >> >> > FAIL: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++98
> >> >> > (test for excess errors)
> >> >> > XPASS: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c
> >> >> > -std=c++14 (internal compiler error)
> >> >> > FAIL: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++14
> >> >> > (test for excess errors)
> >> >> > XPASS: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c
> >> >> > -std=c++17 (internal compiler error)
> >> >> > FAIL: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++17
> >> >> > (test for excess errors)
> >> >> > XPASS: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c
> >> >> > -std=c++20 (internal compiler error)
> >> >> > FAIL: c-c++-common/goacc/kernels-decompose-pr100400-1-2.c -std=c++20
> >> >> > (test for excess errors)
> >> >>
> >> >> Confirmed, and sorry. I had taken care to add explicit '-fchecking'
> >> >> next to 'dg-ice', but that's in fact not the problem/cure here.
> >> >> OK to push to the relevant branches the attached
> >> >> "Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave
> >> >> consistently, regardless of checking level"?
> >> >
> >> > No,
> >> >
> >> > +++ b/gcc/omp-oacc-kernels-decompose.cc
> >> > @@ -239,7 +239,13 @@ visit_loops_in_gang_single_region
> >> > (gimple_stmt_iterator *gsi_p,
> >> > case GIMPLE_OMP_FOR:
> >> > /*TODO Given the current 'adjust_region_code' algorithm, this is
> >> > actually... */
> >> > +#if 0
> >> > gcc_unreachable ();ember all reasons, but parts of them simply
was: new and potentia
> >> > +#else
> >> > + /* ..., but due to bugs (PR100400), we may actually come here.
> >> > + Reliably catch this, regardless of checking level. */
> >> > + abort ();
> >> > +#endif
> >> >
> >> > this doesn't look correct. If you want a reliable diagnostic here
> >> > please use
> >> > sorry ("...") or call internal_error () manually (the IL verifiers do
> >> > this).
> >>
> >> Hmm, I feel I'm going in circles... ;-)
> >>
> >> Here, 'sorry' isn't appropriate, because this is a plain bug, and not
> >> "a language feature which is required by the relevant specification but
> >> not implemented by GCC" ('gcc/diagnostic.cc').
> >>
> >> I first had this as 'internal_error', but then saw the following source
> >> code comment, 'gcc/diagnostic.cc':
> >>
> >> /* An internal consistency check has failed. We make no attempt to
> >> continue. Note that unless there is debugging value to be had from
> >> a more specific message, or some other good reason, you should use
> >> abort () instead of calling this function directly. */
> >> void
> >> internal_error (const char *gmsgid, ...)
> >> {
> >>
> >> Here, there's no "debugging value to be had from a more specific
> >> message", and I couldn't think of "some other good reason", so decided to
> >> "use abort () instead of calling this function directly".
> >
> > I think that is misguided.
>
> So that I know which one to fix/reconsider: does your "that" refer to the
> 'gcc/diagnostic.cc:internal_error' source code comment cited above, or my
> interpretation of it?
The comment to "use abort ()".
> >> But, if that's what we want, I'm happy to again switch to
> >> 'internal_error', and then we should update this source code comment,
> >> too?
>
> > So maybe do
> >
> > internal_error ("PRnnnn");
> >
> > then?
>
> Sure, but note that this just changes from:
>
> [...]
> during GIMPLE pass: omp_oacc_kernels_decompose
> dump file:
> kernels-decompose-pr100400-1-3.c.008t.omp_oacc_kernels_decompose
>
> source-gcc/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-3.c:
> In function ‘void foo()’:
>
> source-gcc/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-3.c:42:7:
> internal compiler error: in visit_loops_in_gang_single_region, at
> omp-oacc-kernels-decompose.cc:247
> 42 | ;
> | ^
> [backtrace]
>
> ... to:
>
> [...]
> during GIMPLE pass: omp_oacc_kernels_decompose
> dump file:
> kernels-decompose-pr100400-1-3.c.008t.omp_oacc_kernels_decompose
>
> source-gcc/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-3.c:
> In function ‘void foo()’:
>
> source-gcc/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-3.c:42:7:
> internal compiler error: PR100400
> 42 | ;
> | ^
> [backtrace]
>
> ..., and it wasn't clear to me that it's an actual improvement to
> diagnose "internal compiler error: PR100400" instead of
> "internal compiler error: in visit_loops_in_gang_single_region, at
> omp-oacc-kernels-decompose.cc:247" (with that location pointing to
> PR100400).
Well, it should be an improvement for consistency? Ah, of course you still
get 'confused by earlier errors' then and a testsuite FAIL.
> That's 'gcc/system.h':
>
> #define abort() fancy_abort (__FILE__, __LINE__, __FUNCTION__)
>
> ..., plus 'gcc/diagnostic.cc':
>
> /* Report an internal compiler error in a friendly manner. This is
> the function that gets called upon use of abort() in the source
> code generally, thanks to a special macro. */
>
> void
> fancy_abort (const char *file, int line, const char *function)
> {
> [...]
> internal_error ("in %s, at %s:%d", function, trim_filename (file),
> line);
> }
>
> Anyway, fine by me, if you like that better? ;-P
>
> OK then to push to the relevant branches the attached
> "Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave
> consistently, regardless of checking level"?
OK for trunk and the GCC 12 branch after the 12.1 release.
Richard.
>
> 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
