From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 48EE83857C4B for ; Tue, 9 Nov 2021 14:38:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 48EE83857C4B Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CEDC42B; Tue, 9 Nov 2021 06:38:13 -0800 (PST) Received: from localhost (unknown [10.32.98.88]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 583363F800; Tue, 9 Nov 2021 06:38:13 -0800 (PST) From: Richard Sandiford To: apinski--- via Gcc-patches Mail-Followup-To: apinski--- via Gcc-patches , apinski@marvell.com, richard.sandiford@arm.com Cc: apinski@marvell.com Subject: Re: [PATCH] Fix aarch64 PR 99657: ICE with SVE types used without an error References: <1636452570-4846-1-git-send-email-apinski@marvell.com> Date: Tue, 09 Nov 2021 14:38:12 +0000 In-Reply-To: <1636452570-4846-1-git-send-email-apinski@marvell.com> (apinski's message of "Tue, 9 Nov 2021 02:09:30 -0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Nov 2021 14:38:18 -0000 apinski--- via Gcc-patches writes: > From: Andrew Pinski > > This fixes fully where SVE types were being used without sve being enabled. > Instead of trying to fix it such that we error out during RTL time, it is > better to error out in front-ends. This expands verify_type_context to > have a context of auto storage decl which is used for both auto storage > decls and for indirection context. > > A few testcases needed to be updated for the new error message; they were > already being rejected before hand. > > OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. > > PR target/99657 > gcc/c/ChangeLog: > > * c-decl.c (finish_decl): Call verify_type_context > for all decls and not just global_decls. > * c-typeck.c (build_indirect_ref): Call verify_type_context > to check to see if the type is ok to be used. > > gcc/ChangeLog: > > * config/aarch64/aarch64-sve-builtins.cc (verify_type_context): > Add TXTC_AUTO_STORAGE support > * target.h (enum type_context_kind): Add TXTC_AUTO_STORAGE. > > gcc/cp/ChangeLog: > > * decl.c (cp_finish_decl): Call verify_type_context > for all decls and not just global_decls. > * typeck.c (cp_build_indirect_ref_1): Call verify_type_context > to check to see if the type is ok to be used. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/sve/acle/general/nosve_1.c: Update test. > * gcc.target/aarch64/sve/acle/general/nosve_4.c: Likewise. > * gcc.target/aarch64/sve/acle/general/nosve_5.c: Likewise. > * gcc.target/aarch64/sve/acle/general/nosve_6.c: Likewise. > * gcc.target/aarch64/sve/pcs/nosve_2.c: Likewise. > * gcc.target/aarch64/sve/pcs/nosve_3.c: Likewise. > * gcc.target/aarch64/sve/pcs/nosve_4.c: Likewise. > * gcc.target/aarch64/sve/pcs/nosve_5.c: Likewise. > * gcc.target/aarch64/sve/pcs/nosve_6.c: Likewise. > * gcc.target/aarch64/sve/pcs/nosve_9.c: New test. The AArch64 bits look good to me. I agree that this gives better error messages than trying to track the error during RTL time (although we still have to do that too in some cases). Thanks, Richard > --- > gcc/c/c-decl.c | 14 +++++++------- > gcc/c/c-typeck.c | 2 ++ > gcc/config/aarch64/aarch64-sve-builtins.cc | 14 ++++++++++++++ > gcc/cp/decl.c | 10 ++++++---- > gcc/cp/typeck.c | 4 ++++ > gcc/target.h | 3 +++ > .../gcc.target/aarch64/sve/acle/general/nosve_1.c | 1 + > .../gcc.target/aarch64/sve/acle/general/nosve_4.c | 2 +- > .../gcc.target/aarch64/sve/acle/general/nosve_5.c | 2 +- > .../gcc.target/aarch64/sve/acle/general/nosve_6.c | 1 + > .../gcc.target/aarch64/sve/pcs/nosve_2.c | 2 +- > .../gcc.target/aarch64/sve/pcs/nosve_3.c | 2 +- > .../gcc.target/aarch64/sve/pcs/nosve_4.c | 3 +-- > .../gcc.target/aarch64/sve/pcs/nosve_5.c | 3 +-- > .../gcc.target/aarch64/sve/pcs/nosve_6.c | 3 +-- > .../gcc.target/aarch64/sve/pcs/nosve_9.c | 15 +++++++++++++++ > 16 files changed, 60 insertions(+), 21 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_9.c > > diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c > index 186fa1692c1..b3583622475 100644 > --- a/gcc/c/c-decl.c > +++ b/gcc/c/c-decl.c > @@ -5441,19 +5441,19 @@ finish_decl (tree decl, location_t init_loc, tree init, > > if (VAR_P (decl)) > { > + type_context_kind context = TCTX_AUTO_STORAGE; > if (init && TREE_CODE (init) == CONSTRUCTOR) > add_flexible_array_elts_to_size (decl, init); > > complete_flexible_array_elts (DECL_INITIAL (decl)); > > if (is_global_var (decl)) > - { > - type_context_kind context = (DECL_THREAD_LOCAL_P (decl) > - ? TCTX_THREAD_STORAGE > - : TCTX_STATIC_STORAGE); > - if (!verify_type_context (input_location, context, TREE_TYPE (decl))) > - TREE_TYPE (decl) = error_mark_node; > - } > + context = (DECL_THREAD_LOCAL_P (decl) > + ? TCTX_THREAD_STORAGE > + : TCTX_STATIC_STORAGE); > + > + if (!verify_type_context (input_location, context, TREE_TYPE (decl))) > + TREE_TYPE (decl) = error_mark_node; > > if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) != error_mark_node > && COMPLETE_TYPE_P (TREE_TYPE (decl))) > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c > index 782414f8c8c..e926b7c1964 100644 > --- a/gcc/c/c-typeck.c > +++ b/gcc/c/c-typeck.c > @@ -2630,6 +2630,8 @@ build_indirect_ref (location_t loc, tree ptr, ref_operator errstring) > else > { > tree t = TREE_TYPE (type); > + if (!verify_type_context (loc, TCTX_AUTO_STORAGE, t)) > + return error_mark_node; > > ref = build1 (INDIRECT_REF, t, pointer); > > diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc > index bc92213665c..1d5083bf9fa 100644 > --- a/gcc/config/aarch64/aarch64-sve-builtins.cc > +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc > @@ -3834,11 +3834,25 @@ bool > verify_type_context (location_t loc, type_context_kind context, > const_tree type, bool silent_p) > { > + static bool informed = false; > if (!sizeless_type_p (type)) > return true; > > switch (context) > { > + case TCTX_AUTO_STORAGE: > + if (TARGET_SVE) > + return true; > + if (!silent_p && !informed) > + { > + informed = true; > + error_at (loc, "SVE type %qT used without SVE ISA enabled", type); > + inform (loc, "you can enable SVE using the command-line" > + " option %<-march%>, or by using the %" > + " attribute or pragma"); > + } > + return false; > + > case TCTX_SIZEOF: > case TCTX_STATIC_STORAGE: > if (!silent_p) > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > index 947bbfc6637..1ffd11acae0 100644 > --- a/gcc/cp/decl.c > +++ b/gcc/cp/decl.c > @@ -7975,11 +7975,13 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p, > && DECL_INITIALIZED_IN_CLASS_P (decl)) > check_static_variable_definition (decl, type); > > - if (!processing_template_decl && VAR_P (decl) && is_global_var (decl)) > + if (!processing_template_decl && VAR_P (decl)) > { > - type_context_kind context = (DECL_THREAD_LOCAL_P (decl) > - ? TCTX_THREAD_STORAGE > - : TCTX_STATIC_STORAGE); > + type_context_kind context = TCTX_AUTO_STORAGE; > + if (is_global_var (decl)) > + context = (DECL_THREAD_LOCAL_P (decl) > + ? TCTX_THREAD_STORAGE > + : TCTX_STATIC_STORAGE); > verify_type_context (input_location, context, TREE_TYPE (decl)); > } > > diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c > index cb20329ceb5..61122e160a9 100644 > --- a/gcc/cp/typeck.c > +++ b/gcc/cp/typeck.c > @@ -3608,6 +3608,10 @@ cp_build_indirect_ref_1 (location_t loc, tree ptr, ref_operator errorstring, > return TREE_OPERAND (pointer, 0); > else > { > + if (!verify_type_context (loc, TCTX_AUTO_STORAGE, t, > + !(complain & tf_error))) > + return error_mark_node; > + > tree ref = build1 (INDIRECT_REF, t, pointer); > > /* We *must* set TREE_READONLY when dereferencing a pointer to const, > diff --git a/gcc/target.h b/gcc/target.h > index d8f45fb99c3..2570b775c5e 100644 > --- a/gcc/target.h > +++ b/gcc/target.h > @@ -217,6 +217,9 @@ const unsigned int VECT_COMPARE_COSTS = 1U << 0; > /* The contexts in which the use of a type T can be checked by > TARGET_VERIFY_TYPE_CONTEXT. */ > enum type_context_kind { > + /* Creeating objects of type T with auto storage duration. */ > + TCTX_AUTO_STORAGE, > + > /* Directly measuring the size of T. */ > TCTX_SIZEOF, > > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_1.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_1.c > index 09dfacd222d..3ea786fe4b3 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_1.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_1.c > @@ -7,6 +7,7 @@ f (svbool_t *x, svint8_t *y) > { > *x = svptrue_b8 (); /* { dg-error {ACLE function '(svbool_t svptrue_b8\(\)|svptrue_b8)' requires ISA extension 'sve'} } */ > /* { dg-message {note: you can enable 'sve' using the command-line option '-march', or by using the 'target' attribute or pragma} "" { target *-*-* } .-1 } */ > + /* { dg-error "SVE type 'svbool_t' {aka '__SVBool_t'} used without SVE ISA enabled" "indirect" {target *-*-* } .-2 } */ > *x = svptrue_b8 (); > *x = svptrue_b8 (); > *x = svptrue_b8 (); > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_4.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_4.c > index 35ab07f1b49..6aef1a77f14 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_4.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_4.c > @@ -3,6 +3,6 @@ > void > f (__SVBool_t *x, __SVBool_t *y) > { > - *x = *y; /* { dg-error {this operation requires the SVE ISA extension} } */ > + *x = *y; /* { dg-error {SVE type '__SVBool_t' used without SVE ISA enabled} } */ > *x = *y; > } > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_5.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_5.c > index 6e8d951b294..33e2069fcb0 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_5.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_5.c > @@ -3,6 +3,6 @@ > void > f (__SVInt8_t *x, __SVInt8_t *y) > { > - *x = *y; /* { dg-error {this operation requires the SVE ISA extension} } */ > + *x = *y; /* { dg-error {SVE type '__SVInt8_t' used without SVE ISA enabled} } */ > *x = *y; > } > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_6.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_6.c > index d91ba40de14..7dc10528a17 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_6.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/nosve_6.c > @@ -8,5 +8,6 @@ void > f (svbool_t *x, svint8_t *y) > { > *x = svptrue_b8 (); /* { dg-error {ACLE function '(svbool_t svptrue_b8\(\)|svptrue_b8)' is incompatible with the use of '-mgeneral-regs-only'} } */ > + /* { dg-error {SVE type 'svbool_t' {aka '__SVBool_t'} used without SVE ISA enabled} "" { target *-*-* } .-1 } */ > *y = svadd_m (*x, *y, 1); > } > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_2.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_2.c > index 663165f892d..0f1bad1734e 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_2.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_2.c > @@ -10,5 +10,5 @@ svbool_t return_bool (); > void > f (svbool_t *ptr) > { > - *ptr = return_bool (); /* { dg-error {'return_bool' requires the SVE ISA extension} } */ > + *ptr = return_bool (); /* { dg-error {SVE type 'svbool_t' used without SVE ISA enabled} } */ > } > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_3.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_3.c > index 6d5823cfde1..2f9ebd827a7 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_3.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_3.c > @@ -10,5 +10,5 @@ svbool_t (*return_bool) (); > void > f (svbool_t *ptr) > { > - *ptr = return_bool (); /* { dg-error {calls to functions of type 'svbool_t\(\)' require the SVE ISA extension} } */ > + *ptr = return_bool (); /* { dg-error {SVE type 'svbool_t' used without SVE ISA enabled} } */ > } > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_4.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_4.c > index a248bdbdbd9..979f98c11c8 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_4.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_4.c > @@ -10,6 +10,5 @@ void take_svuint8 (svuint8_t); > void > f (svuint8_t *ptr) > { > - take_svuint8 (*ptr); /* { dg-error {this operation requires the SVE ISA extension} } */ > - /* { dg-error {'take_svuint8' requires the SVE ISA extension} "" { target *-*-* } .-1 } */ > + take_svuint8 (*ptr); /* { dg-error {SVE type 'svuint8_t' used without SVE ISA enabled} } */ > } > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_5.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_5.c > index 6263b5acdec..990e28aae70 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_5.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_5.c > @@ -11,6 +11,5 @@ void take_svuint8_eventually (float, float, float, float, > void > f (svuint8_t *ptr) > { > - take_svuint8_eventually (0, 0, 0, 0, 0, 0, 0, 0, *ptr); /* { dg-error {this operation requires the SVE ISA extension} } */ > - /* { dg-error {arguments of type '(svuint8_t|__SVUint8_t)' require the SVE ISA extension} "" { target *-*-* } .-1 } */ > + take_svuint8_eventually (0, 0, 0, 0, 0, 0, 0, 0, *ptr); /* { dg-error {SVE type 'svuint8_t' used without SVE ISA enabled} } */ > } > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_6.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_6.c > index 85b68bb3881..f11de7017a2 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_6.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_6.c > @@ -10,6 +10,5 @@ void unprototyped (); > void > f (svuint8_t *ptr) > { > - unprototyped (*ptr); /* { dg-error {this operation requires the SVE ISA extension} } */ > - /* { dg-error {arguments of type '(svuint8_t|__SVUint8_t)' require the SVE ISA extension} "" { target *-*-* } .-1 } */ > + unprototyped (*ptr); /* { dg-error {SVE type 'svuint8_t' used without SVE ISA enabled} } */ > } > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_9.c b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_9.c > new file mode 100644 > index 00000000000..7b7f322fe8c > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pcs/nosve_9.c > @@ -0,0 +1,15 @@ > +/* { dg-do compile } */ > +/* { dg-prune-output "compilation terminated" } */ > + > +#include > + > +#pragma GCC target "+nosve" > + > +int > +f () > +{ > + char a[12]; > + __SVInt8_t freq; /* { dg-error {SVE type '__SVInt8_t' used without SVE ISA enabled} } */ > + return __builtin_bcmp (&freq, &freq, 10); > +} > +