"Kewen.Lin" <li...@linux.ibm.com> writes:
> Hi,
>
> In the recent discussion on how to make some built-in type only valid for
> some target features efficiently[1], Andrew mentioned this patch which he
> made previously (Thanks!). I confirmed it can help rs6000 related issue,
> and noticed PR99657 is still opened, so I think we still want this to
> be reviewed.
But does it work for things like:
void f(foo_t *x, foo_t *y) { *x = *y; }
where no variables are being created with foo_t type?
That's not to say we shouldn't have the patch. I'm just not sure
it can be the complete solution.
Thanks,
Richard
>
> Could some C/C++ FE experts help to review it?
>
> Thanks in advance!
>
> BR,
> Kewen
>
> [1] https://gcc.gnu.org/pipermail/gcc/2022-December/240220.html
>
> on 2021/11/9 18:09, apinski--- via Gcc-patches wrote:
>> From: Andrew Pinski <apin...@marvell.com>
>>
>> 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.
>> ---
>> 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 %<target%>"
>> + " 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 <arm_sve.h>
>> +
>> +#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);
>> +}
>> +
