* Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level (was: GCC 12.1 Release Candidate available from gcc.gnu.org) [not found] ` <BC3318BA-D91C-4357-9015-1538A112807A@googlemail.com> @ 2022-05-02 14:01 ` Thomas Schwinge 2022-05-03 7:17 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Thomas Schwinge @ 2022-05-02 14:01 UTC (permalink / raw) To: Iain Sandoe, Jakub Jelinek, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1904 bytes --] Hi! On 2022-05-01T11:02:29+0100, Iain Sandoe via Gcc <gcc@gcc.gnu.org> wrote: >> On 29 Apr 2022, at 15:34, Jakub Jelinek via Gcc <gcc@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"? 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Make-c-c-common-goacc-kernels-decompose-pr100400-1-..patch --] [-- Type: text/x-diff, Size: 5229 bytes --] From 7827d018a9e0109b8d271ceb824f2c718bae508e Mon Sep 17 00:00:00 2001 From: Thomas Schwinge <thomas@codesourcery.com> Date: Mon, 2 May 2022 15:15:26 +0200 Subject: [PATCH] Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level Fix-up for commit c14ea6a72fb1ae66e3d32ac8329558497c6e4403 "Catch 'GIMPLE_DEBUG' misbehavior in OpenACC 'kernels' decomposition [PR100400, PR103836, PR104061]". For C++ compilation of 'c-c++-common/goacc/kernels-decompose-pr100400-1-2.c', we first emit a 'sorry' diagnostic, and then a 'gcc_unreachable' (or 'abort', see below) diagnostic, but for example, for '--enable-checking=release' (thus, '!CHECKING_P'), the second one may actually be turned into a 'confused by earlier errors, bailing out' diagnostic. (See 'gcc/diagnostic.cc:diagnostic_report_diagnostic': "When not checking, ICEs are converted to fatal errors when an error has already occurred.") Thus, make 'c-c++-common/goacc/kernels-decompose-pr100400-1-2.c' behave consistently via '-Wfatal-errors', and thus only matching the 'sorry' diagnostic. For example, for '--enable-checking=no' (thus, '!ENABLE_ASSERT_CHECKING'), a call to 'gcc_unreachable' cannot be assumed emit an 'abort'-like diagnostic, so explicitly call 'abort' in 'gcc/omp-oacc-kernels-decompose.cc:visit_loops_in_gang_single_region', in the 'GIMPLE_OMP_FOR' case, to avoid regressing 'c-c++-common/goacc/kernels-decompose-pr100400-1-3.c', and 'c-c++-common/goacc/kernels-decompose-pr100400-1-4.c'. PR middle-end/100400 gcc/ * omp-oacc-kernels-decompose.cc (visit_loops_in_gang_single_region) <GIMPLE_OMP_FOR>: Reliably 'abort'. gcc/testsuite/ * c-c++-common/goacc/kernels-decompose-pr100400-1-2.c: Specify '-Wfatal-errors'. --- gcc/omp-oacc-kernels-decompose.cc | 6 ++++++ .../goacc/kernels-decompose-pr100400-1-2.c | 12 +++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/gcc/omp-oacc-kernels-decompose.cc b/gcc/omp-oacc-kernels-decompose.cc index 4386787ba3c..9f5c1ecb255 100644 --- a/gcc/omp-oacc-kernels-decompose.cc +++ 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 (); +#else + /* ..., but due to bugs (PR100400), we may actually come here. + Reliably catch this, regardless of checking level. */ + abort (); +#endif { tree clauses = gimple_omp_for_clauses (stmt); diff --git a/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c b/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c index a643f109bf1..8b65e07c623 100644 --- a/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c +++ b/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c @@ -1,8 +1,8 @@ /* { dg-additional-options "--param openacc-kernels=decompose" } */ -/* { dg-additional-options "-fchecking" } - { dg-ice TODO { c++ } } - { dg-prune-output "during GIMPLE pass: omp_oacc_kernels_decompose" } */ +/* Ensure consistent diagnostics, regardless of checking level: + { dg-additional-options -Wfatal-errors } + { dg-message {terminated due to -Wfatal-errors} TODO { target *-*-* } 0 } */ /* { dg-additional-options "-g" } */ /* { dg-additional-options "-O1" } so that we may get some 'GIMPLE_DEBUG's. */ @@ -19,18 +19,16 @@ foo (void) /* { dg-bogus {sorry, unimplemented: 'gimple_debug' not yet supported} TODO { xfail *-*-* } .+1 } */ #pragma acc kernels /* { dg-line l_compute1 } */ /* { dg-note {OpenACC 'kernels' decomposition: variable 'p' in 'copy' clause requested to be made addressable} {} { target *-*-* } l_compute1 } - { dg-note {variable 'p' made addressable} {} { target *-*-* xfail c++ } l_compute1 } */ + { dg-note {variable 'p' made addressable} {} { xfail *-*-* } l_compute1 } */ /* { dg-note {variable 'c' declared in block is candidate for adjusting OpenACC privatization level} {} { xfail *-*-* } l_compute1 } */ /* { dg-note {variable 'c\.0' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} {} { xfail *-*-* } l_compute1 } */ { - /* { dg-bogus {note: beginning 'gang-single' part in OpenACC 'kernels' region} {w/ debug} { xfail c++ } .-1 } - { dg-bogus {note: beginning 'gang-single' part in OpenACC 'kernels' region} {w/ debug} { xfail c } .+1 } */ int c; /* { dg-note {beginning 'gang-single' part in OpenACC 'kernels' region} {} { xfail *-*-* } .+1 } */ p = &c; - /* { dg-note {parallelized loop nest in OpenACC 'kernels' region} {} { xfail c++ } .+1 } */ + /* { dg-note {parallelized loop nest in OpenACC 'kernels' region} {} { xfail *-*-* } .+1 } */ #pragma acc loop independent /* { dg-line l_loop_c1 } */ /* { dg-note {variable 'c\.0' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} {} { xfail *-*-* } l_loop_c1 } */ /* { dg-note {variable 'c' in 'private' clause is candidate for adjusting OpenACC privatization level} {} { xfail *-*-* } l_loop_c1 } -- 2.35.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level (was: GCC 12.1 Release Candidate available from gcc.gnu.org) 2022-05-02 14:01 ` Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level (was: GCC 12.1 Release Candidate available from gcc.gnu.org) Thomas Schwinge @ 2022-05-03 7:17 ` Richard Biener 2022-05-03 8:16 ` Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level Thomas Schwinge 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2022-05-03 7:17 UTC (permalink / raw) To: Thomas Schwinge; +Cc: Iain Sandoe, Jakub Jelinek, GCC Patches On Mon, May 2, 2022 at 4:01 PM Thomas Schwinge <thomas@codesourcery.com> wrote: > > Hi! > > On 2022-05-01T11:02:29+0100, Iain Sandoe via Gcc <gcc@gcc.gnu.org> wrote: > >> On 29 Apr 2022, at 15:34, Jakub Jelinek via Gcc <gcc@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 (); +#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). That said, having a testcase that checks for an ICE as a TODO is maybe not the very best thing to have. We have bugzilla for unfixed bugs. 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level 2022-05-03 7:17 ` Richard Biener @ 2022-05-03 8:16 ` Thomas Schwinge 2022-05-03 10:53 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Thomas Schwinge @ 2022-05-03 8:16 UTC (permalink / raw) To: Richard Biener; +Cc: Iain Sandoe, Jakub Jelinek, gcc-patches Hi Richard! On 2022-05-03T09:17:52+0200, Richard Biener <richard.guenther@gmail.com> wrote: > On Mon, May 2, 2022 at 4:01 PM Thomas Schwinge <thomas@codesourcery.com> wrote: >> On 2022-05-01T11:02:29+0100, Iain Sandoe via Gcc <gcc@gcc.gnu.org> wrote: >> >> On 29 Apr 2022, at 15:34, Jakub Jelinek via Gcc <gcc@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 (); > +#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". 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? > That said, having a testcase that checks for an ICE as a TODO is maybe not > the very best thing to have. We have bugzilla for unfixed bugs. As per the past 'dg-ice' discussions, there's certainly value seen for having such test cases (and it already did help me during development, elsewhere). I do agree/acknowledge that it's a bit more difficult to make those behave consistently across GCC configurations. 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level 2022-05-03 8:16 ` Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level Thomas Schwinge @ 2022-05-03 10:53 ` Richard Biener 2022-05-03 12:29 ` Thomas Schwinge 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2022-05-03 10:53 UTC (permalink / raw) To: Thomas Schwinge; +Cc: Iain Sandoe, Jakub Jelinek, GCC Patches On Tue, May 3, 2022 at 10:16 AM Thomas Schwinge <thomas@codesourcery.com> wrote: > > Hi Richard! > > On 2022-05-03T09:17:52+0200, Richard Biener <richard.guenther@gmail.com> wrote: > > On Mon, May 2, 2022 at 4:01 PM Thomas Schwinge <thomas@codesourcery.com> wrote: > >> On 2022-05-01T11:02:29+0100, Iain Sandoe via Gcc <gcc@gcc.gnu.org> wrote: > >> >> On 29 Apr 2022, at 15:34, Jakub Jelinek via Gcc <gcc@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 (); > > +#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. > 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? > > > > That said, having a testcase that checks for an ICE as a TODO is maybe not > > the very best thing to have. We have bugzilla for unfixed bugs. > > As per the past 'dg-ice' discussions, there's certainly value seen for > having such test cases (and it already did help me during development, > elsewhere). > > I do agree/acknowledge that it's a bit more difficult to make those > behave consistently across GCC configurations. So maybe do internal_error ("PRnnnn"); then? > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level 2022-05-03 10:53 ` Richard Biener @ 2022-05-03 12:29 ` Thomas Schwinge 2022-05-03 13:46 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Thomas Schwinge @ 2022-05-03 12:29 UTC (permalink / raw) To: Richard Biener, gcc-patches; +Cc: Iain Sandoe, Jakub Jelinek [-- Attachment #1: Type: text/plain, Size: 6530 bytes --] Hi Richard! On 2022-05-03T12:53:50+0200, Richard Biener <richard.guenther@gmail.com> wrote: > On Tue, May 3, 2022 at 10:16 AM Thomas Schwinge <thomas@codesourcery.com> wrote: >> On 2022-05-03T09:17:52+0200, Richard Biener <richard.guenther@gmail.com> wrote: >> > On Mon, May 2, 2022 at 4:01 PM Thomas Schwinge <thomas@codesourcery.com> wrote: >> >> On 2022-05-01T11:02:29+0100, Iain Sandoe via Gcc <gcc@gcc.gnu.org> wrote: >> >> >> On 29 Apr 2022, at 15:34, Jakub Jelinek via Gcc <gcc@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 (); >> > +#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? >> 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). 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"? 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Make-c-c-common-goacc-kernels-decompose-pr100400-1-..patch --] [-- Type: text/x-diff, Size: 5291 bytes --] From ff9e80d1aa257ec53637b14641c2b3027dbf7675 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge <thomas@codesourcery.com> Date: Mon, 2 May 2022 15:15:26 +0200 Subject: [PATCH] Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level Fix-up for commit c14ea6a72fb1ae66e3d32ac8329558497c6e4403 "Catch 'GIMPLE_DEBUG' misbehavior in OpenACC 'kernels' decomposition [PR100400, PR103836, PR104061]". For C++ compilation of 'c-c++-common/goacc/kernels-decompose-pr100400-1-2.c', we first emit a 'sorry' diagnostic, and then a 'gcc_unreachable' (or 'internal_error', see below) diagnostic, but for example, for '--enable-checking=release' (thus, '!CHECKING_P'), the second one may actually be turned into a 'confused by earlier errors, bailing out' diagnostic. (See 'gcc/diagnostic.cc:diagnostic_report_diagnostic': "When not checking, ICEs are converted to fatal errors when an error has already occurred.") Thus, make 'c-c++-common/goacc/kernels-decompose-pr100400-1-2.c' behave consistently via '-Wfatal-errors', and thus only matching the 'sorry' diagnostic. For example, for '--enable-checking=no' (thus, '!ENABLE_ASSERT_CHECKING'), a call to 'gcc_unreachable' cannot be assumed emit an 'internal_error'-like diagnostic, so explicitly call 'internal_error' in 'gcc/omp-oacc-kernels-decompose.cc:visit_loops_in_gang_single_region', in the 'GIMPLE_OMP_FOR' case, to avoid regressing 'c-c++-common/goacc/kernels-decompose-pr100400-1-3.c', and 'c-c++-common/goacc/kernels-decompose-pr100400-1-4.c'. PR middle-end/100400 gcc/ * omp-oacc-kernels-decompose.cc (visit_loops_in_gang_single_region) <GIMPLE_OMP_FOR>: Explicitly call 'internal_error'. gcc/testsuite/ * c-c++-common/goacc/kernels-decompose-pr100400-1-2.c: Specify '-Wfatal-errors'. --- gcc/omp-oacc-kernels-decompose.cc | 6 ++++++ .../goacc/kernels-decompose-pr100400-1-2.c | 12 +++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/gcc/omp-oacc-kernels-decompose.cc b/gcc/omp-oacc-kernels-decompose.cc index 4386787ba3c..ec9b0faab0a 100644 --- a/gcc/omp-oacc-kernels-decompose.cc +++ 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 (); +#else + /* ..., but due to bugs (PR100400), we may actually come here. + Reliably catch this, regardless of checking level. */ + internal_error ("PR100400"); +#endif { tree clauses = gimple_omp_for_clauses (stmt); diff --git a/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c b/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c index a643f109bf1..8b65e07c623 100644 --- a/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c +++ b/gcc/testsuite/c-c++-common/goacc/kernels-decompose-pr100400-1-2.c @@ -1,8 +1,8 @@ /* { dg-additional-options "--param openacc-kernels=decompose" } */ -/* { dg-additional-options "-fchecking" } - { dg-ice TODO { c++ } } - { dg-prune-output "during GIMPLE pass: omp_oacc_kernels_decompose" } */ +/* Ensure consistent diagnostics, regardless of checking level: + { dg-additional-options -Wfatal-errors } + { dg-message {terminated due to -Wfatal-errors} TODO { target *-*-* } 0 } */ /* { dg-additional-options "-g" } */ /* { dg-additional-options "-O1" } so that we may get some 'GIMPLE_DEBUG's. */ @@ -19,18 +19,16 @@ foo (void) /* { dg-bogus {sorry, unimplemented: 'gimple_debug' not yet supported} TODO { xfail *-*-* } .+1 } */ #pragma acc kernels /* { dg-line l_compute1 } */ /* { dg-note {OpenACC 'kernels' decomposition: variable 'p' in 'copy' clause requested to be made addressable} {} { target *-*-* } l_compute1 } - { dg-note {variable 'p' made addressable} {} { target *-*-* xfail c++ } l_compute1 } */ + { dg-note {variable 'p' made addressable} {} { xfail *-*-* } l_compute1 } */ /* { dg-note {variable 'c' declared in block is candidate for adjusting OpenACC privatization level} {} { xfail *-*-* } l_compute1 } */ /* { dg-note {variable 'c\.0' declared in block isn't candidate for adjusting OpenACC privatization level: not addressable} {} { xfail *-*-* } l_compute1 } */ { - /* { dg-bogus {note: beginning 'gang-single' part in OpenACC 'kernels' region} {w/ debug} { xfail c++ } .-1 } - { dg-bogus {note: beginning 'gang-single' part in OpenACC 'kernels' region} {w/ debug} { xfail c } .+1 } */ int c; /* { dg-note {beginning 'gang-single' part in OpenACC 'kernels' region} {} { xfail *-*-* } .+1 } */ p = &c; - /* { dg-note {parallelized loop nest in OpenACC 'kernels' region} {} { xfail c++ } .+1 } */ + /* { dg-note {parallelized loop nest in OpenACC 'kernels' region} {} { xfail *-*-* } .+1 } */ #pragma acc loop independent /* { dg-line l_loop_c1 } */ /* { dg-note {variable 'c\.0' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} {} { xfail *-*-* } l_loop_c1 } */ /* { dg-note {variable 'c' in 'private' clause is candidate for adjusting OpenACC privatization level} {} { xfail *-*-* } l_loop_c1 } -- 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level 2022-05-03 12:29 ` Thomas Schwinge @ 2022-05-03 13:46 ` Richard Biener 2022-05-10 14:03 ` Advise to call 'internal_error' instead of 'abort' or 'fancy_abort' Thomas Schwinge 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2022-05-03 13:46 UTC (permalink / raw) To: Thomas Schwinge; +Cc: GCC Patches, Iain Sandoe, Jakub Jelinek On Tue, May 3, 2022 at 2:29 PM Thomas Schwinge <thomas@codesourcery.com> wrote: > > Hi Richard! > > On 2022-05-03T12:53:50+0200, Richard Biener <richard.guenther@gmail.com> wrote: > > On Tue, May 3, 2022 at 10:16 AM Thomas Schwinge <thomas@codesourcery.com> wrote: > >> On 2022-05-03T09:17:52+0200, Richard Biener <richard.guenther@gmail.com> wrote: > >> > On Mon, May 2, 2022 at 4:01 PM Thomas Schwinge <thomas@codesourcery.com> wrote: > >> >> On 2022-05-01T11:02:29+0100, Iain Sandoe via Gcc <gcc@gcc.gnu.org> wrote: > >> >> >> On 29 Apr 2022, at 15:34, Jakub Jelinek via Gcc <gcc@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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Advise to call 'internal_error' instead of 'abort' or 'fancy_abort' 2022-05-03 13:46 ` Richard Biener @ 2022-05-10 14:03 ` Thomas Schwinge 2022-05-17 10:15 ` [PING] " Thomas Schwinge 0 siblings, 1 reply; 9+ messages in thread From: Thomas Schwinge @ 2022-05-10 14:03 UTC (permalink / raw) To: Richard Biener, gcc-patches [-- Attachment #1: Type: text/plain, Size: 2553 bytes --] Hi! On 2022-05-03T15:46:43+0200, Richard Biener <richard.guenther@gmail.com> wrote: > On Tue, May 3, 2022 at 2:29 PM Thomas Schwinge <thomas@codesourcery.com> wrote: >> On 2022-05-03T12:53:50+0200, Richard Biener <richard.guenther@gmail.com> wrote: >> > On Tue, May 3, 2022 at 10:16 AM Thomas Schwinge <thomas@codesourcery.com> wrote: >> >> On 2022-05-03T09:17:52+0200, Richard Biener <richard.guenther@gmail.com> wrote: >> >> > On Mon, May 2, 2022 at 4:01 PM Thomas Schwinge <thomas@codesourcery.com> wrote: >> >> > +#if 0 >> >> > gcc_unreachable (); >> >> > +#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 [...] >> >> > call internal_error () manually (the IL verifiers do this). >> >> >> >> Hmm, I feel I'm going in circles... ;-) >> >> 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 ()". Does the attached "Advise to call 'internal_error' instead of 'abort' or 'fancy_abort'" capture what you had in mind? (This is, obviously, not updating any of the many 'abort' or even a few 'fancy_abort' calls that we currently have.) 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Advise-to-call-internal_error-instead-of-abort-or-fa.patch --] [-- Type: text/x-diff, Size: 1930 bytes --] From a8017c7b5fa7b5e8210b6446acf7dd09989a7517 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge <thomas@codesourcery.com> Date: Tue, 10 May 2022 15:56:08 +0200 Subject: [PATCH] Advise to call 'internal_error' instead of 'abort' or 'fancy_abort' gcc/ * diagnostic.cc: Don't advise to call 'abort' instead of 'internal_error'. * system.h: Advise to call 'internal_error' instead of 'abort' or 'fancy_abort'. Suggested-by: Richard Biener <richard.guenther@gmail.com> --- gcc/diagnostic.cc | 4 +--- gcc/system.h | 6 ++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc index 73324a728fe..fef11467b6f 100644 --- a/gcc/diagnostic.cc +++ b/gcc/diagnostic.cc @@ -1935,9 +1935,7 @@ fatal_error (location_t loc, const char *gmsgid, ...) } /* 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. */ + continue. */ void internal_error (const char *gmsgid, ...) { diff --git a/gcc/system.h b/gcc/system.h index c25cd64366f..187763efcd6 100644 --- a/gcc/system.h +++ b/gcc/system.h @@ -770,8 +770,10 @@ extern int vsnprintf (char *, size_t, const char *, va_list); #endif #endif -/* Redefine abort to report an internal error w/o coredump, and - reporting the location of the error in the source file. */ +/* Redefine 'abort' to report an internal error w/o coredump, and + reporting the location of the error in the source file. + Instead of directly calling 'abort' or 'fancy_abort', GCC code + should normally call 'internal_error' with a specific message. */ extern void fancy_abort (const char *, int, const char *) ATTRIBUTE_NORETURN ATTRIBUTE_COLD; #define abort() fancy_abort (__FILE__, __LINE__, __FUNCTION__) -- 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PING] Advise to call 'internal_error' instead of 'abort' or 'fancy_abort' 2022-05-10 14:03 ` Advise to call 'internal_error' instead of 'abort' or 'fancy_abort' Thomas Schwinge @ 2022-05-17 10:15 ` Thomas Schwinge 2022-05-17 14:13 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Thomas Schwinge @ 2022-05-17 10:15 UTC (permalink / raw) To: Richard Biener, gcc-patches [-- Attachment #1: Type: text/plain, Size: 2695 bytes --] Hi! Ping. Grüße Thomas On 2022-05-10T16:03:12+0200, I wrote: > Hi! > > On 2022-05-03T15:46:43+0200, Richard Biener <richard.guenther@gmail.com> wrote: >> On Tue, May 3, 2022 at 2:29 PM Thomas Schwinge <thomas@codesourcery.com> wrote: >>> On 2022-05-03T12:53:50+0200, Richard Biener <richard.guenther@gmail.com> wrote: >>> > On Tue, May 3, 2022 at 10:16 AM Thomas Schwinge <thomas@codesourcery.com> wrote: >>> >> On 2022-05-03T09:17:52+0200, Richard Biener <richard.guenther@gmail.com> wrote: >>> >> > On Mon, May 2, 2022 at 4:01 PM Thomas Schwinge <thomas@codesourcery.com> wrote: >>> >> > +#if 0 >>> >> > gcc_unreachable (); >>> >> > +#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 [...] >>> >> > call internal_error () manually (the IL verifiers do this). >>> >> >>> >> Hmm, I feel I'm going in circles... ;-) > >>> >> 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 ()". > > Does the attached > "Advise to call 'internal_error' instead of 'abort' or 'fancy_abort'" > capture what you had in mind? > > (This is, obviously, not updating any of the many 'abort' or even a few > 'fancy_abort' calls that we currently have.) > > > 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Advise-to-call-internal_error-instead-of-abort-or-fa.patch --] [-- Type: text/x-diff, Size: 1930 bytes --] From a8017c7b5fa7b5e8210b6446acf7dd09989a7517 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge <thomas@codesourcery.com> Date: Tue, 10 May 2022 15:56:08 +0200 Subject: [PATCH] Advise to call 'internal_error' instead of 'abort' or 'fancy_abort' gcc/ * diagnostic.cc: Don't advise to call 'abort' instead of 'internal_error'. * system.h: Advise to call 'internal_error' instead of 'abort' or 'fancy_abort'. Suggested-by: Richard Biener <richard.guenther@gmail.com> --- gcc/diagnostic.cc | 4 +--- gcc/system.h | 6 ++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc index 73324a728fe..fef11467b6f 100644 --- a/gcc/diagnostic.cc +++ b/gcc/diagnostic.cc @@ -1935,9 +1935,7 @@ fatal_error (location_t loc, const char *gmsgid, ...) } /* 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. */ + continue. */ void internal_error (const char *gmsgid, ...) { diff --git a/gcc/system.h b/gcc/system.h index c25cd64366f..187763efcd6 100644 --- a/gcc/system.h +++ b/gcc/system.h @@ -770,8 +770,10 @@ extern int vsnprintf (char *, size_t, const char *, va_list); #endif #endif -/* Redefine abort to report an internal error w/o coredump, and - reporting the location of the error in the source file. */ +/* Redefine 'abort' to report an internal error w/o coredump, and + reporting the location of the error in the source file. + Instead of directly calling 'abort' or 'fancy_abort', GCC code + should normally call 'internal_error' with a specific message. */ extern void fancy_abort (const char *, int, const char *) ATTRIBUTE_NORETURN ATTRIBUTE_COLD; #define abort() fancy_abort (__FILE__, __LINE__, __FUNCTION__) -- 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PING] Advise to call 'internal_error' instead of 'abort' or 'fancy_abort' 2022-05-17 10:15 ` [PING] " Thomas Schwinge @ 2022-05-17 14:13 ` Richard Biener 0 siblings, 0 replies; 9+ messages in thread From: Richard Biener @ 2022-05-17 14:13 UTC (permalink / raw) To: Thomas Schwinge; +Cc: GCC Patches On Tue, May 17, 2022 at 12:15 PM Thomas Schwinge <thomas@codesourcery.com> wrote: > > Hi! > > Ping. OK. > > Grüße > Thomas > > > On 2022-05-10T16:03:12+0200, I wrote: > > Hi! > > > > On 2022-05-03T15:46:43+0200, Richard Biener <richard.guenther@gmail.com> wrote: > >> On Tue, May 3, 2022 at 2:29 PM Thomas Schwinge <thomas@codesourcery.com> wrote: > >>> On 2022-05-03T12:53:50+0200, Richard Biener <richard.guenther@gmail.com> wrote: > >>> > On Tue, May 3, 2022 at 10:16 AM Thomas Schwinge <thomas@codesourcery.com> wrote: > >>> >> On 2022-05-03T09:17:52+0200, Richard Biener <richard.guenther@gmail.com> wrote: > >>> >> > On Mon, May 2, 2022 at 4:01 PM Thomas Schwinge <thomas@codesourcery.com> wrote: > >>> >> > +#if 0 > >>> >> > gcc_unreachable (); > >>> >> > +#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 [...] > >>> >> > call internal_error () manually (the IL verifiers do this). > >>> >> > >>> >> Hmm, I feel I'm going in circles... ;-) > > > >>> >> 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 ()". > > > > Does the attached > > "Advise to call 'internal_error' instead of 'abort' or 'fancy_abort'" > > capture what you had in mind? > > > > (This is, obviously, not updating any of the many 'abort' or even a few > > 'fancy_abort' calls that we currently have.) > > > > > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-05-17 14:13 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <Ymv3hkP1s1Zc2hj8@tucnak> [not found] ` <BC3318BA-D91C-4357-9015-1538A112807A@googlemail.com> 2022-05-02 14:01 ` Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level (was: GCC 12.1 Release Candidate available from gcc.gnu.org) Thomas Schwinge 2022-05-03 7:17 ` Richard Biener 2022-05-03 8:16 ` Make 'c-c++-common/goacc/kernels-decompose-pr100400-1-*.c' behave consistently, regardless of checking level Thomas Schwinge 2022-05-03 10:53 ` Richard Biener 2022-05-03 12:29 ` Thomas Schwinge 2022-05-03 13:46 ` Richard Biener 2022-05-10 14:03 ` Advise to call 'internal_error' instead of 'abort' or 'fancy_abort' Thomas Schwinge 2022-05-17 10:15 ` [PING] " Thomas Schwinge 2022-05-17 14:13 ` Richard Biener
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).