Re: [PATCH 1/2] Change the name of array_at_struct_end_p to array_ref_flexible_size_p
Thanks.
Committed as: https://gcc.gnu.org/pipermail/gcc-cvs/2022-November/373830.html
Qing
> On Nov 9, 2022, at 2:57 AM, Richard Biener wrote:
>
> On Tue, 8 Nov 2022, Qing Zhao wrote:
>
>> The name of the utility routine "array_at_struct_end_p" is misleading
>> and should be changed to a new name that more accurately reflects its
>> real meaning.
>>
>> The routine "array_at_struct_end_p" is used to check whether an array
>> reference is to an array whose actual size might be larger than its
>> upper bound implies, which includes 3 different cases:
>>
>> A. a ref to a flexible array member at the end of a structure;
>> B. a ref to an array with a different type against the original decl;
>> C. a ref to an array that was passed as a parameter;
>>
>> The old name only reflects the above case A, therefore very confusing
>> when reading the corresponding gcc source code.
>>
>> In this patch, A new name "array_ref_flexible_size_p" is used to replace
>> the old name.
>>
>> All the references to the routine "array_at_struct_end_p" was replaced
>> with this new name, and the corresponding comments were updated to make
>> them clean and consistent.
>
> Since you seem to feel strongly about this - OK.
>
> Thanks,
> Richard.
>
>> gcc/ChangeLog:
>>
>> * gimple-array-bounds.cc (trailing_array): Replace
>> array_at_struct_end_p with new name and update comments.
>> * gimple-fold.cc (get_range_strlen_tree): Likewise.
>> * gimple-ssa-warn-restrict.cc (builtin_memref::builtin_memref):
>> Likewise.
>> * graphite-sese-to-poly.cc (bounds_are_valid): Likewise.
>> * tree-if-conv.cc (idx_within_array_bound): Likewise.
>> * tree-object-size.cc (addr_object_size): Likewise.
>> * tree-ssa-alias.cc (component_ref_to_zero_sized_trailing_array_p):
>> Likewise.
>> (stmt_kills_ref_p): Likewise.
>> * tree-ssa-loop-niter.cc (idx_infer_loop_bounds): Likewise.
>> * tree-ssa-strlen.cc (maybe_set_strlen_range): Likewise.
>> * tree.cc (array_at_struct_end_p): Rename to ...
>> (array_ref_flexible_size_p): ... this.
>> (component_ref_size): Replace array_at_struct_end_p with new name.
>> * tree.h (array_at_struct_end_p): Rename to ...
>> (array_ref_flexible_size_p): ... this.
>> ---
>> gcc/gimple-array-bounds.cc | 4 ++--
>> gcc/gimple-fold.cc | 6 ++
>> gcc/gimple-ssa-warn-restrict.cc | 5 +++--
>> gcc/graphite-sese-to-poly.cc| 4 ++--
>> gcc/tree-if-conv.cc | 7 +++
>> gcc/tree-object-size.cc | 2 +-
>> gcc/tree-ssa-alias.cc | 8
>> gcc/tree-ssa-loop-niter.cc | 15 +++
>> gcc/tree-ssa-strlen.cc | 2 +-
>> gcc/tree.cc | 11 ++-
>> gcc/tree.h | 8
>> 11 files changed, 35 insertions(+), 37 deletions(-)
>>
>> diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
>> index e190b93aa85..fbf448e045d 100644
>> --- a/gcc/gimple-array-bounds.cc
>> +++ b/gcc/gimple-array-bounds.cc
>> @@ -129,7 +129,7 @@ get_ref_size (tree arg, tree *pref)
>> }
>>
>> /* Return true if REF is (likely) an ARRAY_REF to a trailing array member
>> - of a struct. It refines array_at_struct_end_p by detecting a pointer
>> + of a struct. It refines array_ref_flexible_size_p by detecting a pointer
>>to an array and an array parameter declared using the [N] syntax (as
>>opposed to a pointer) and returning false. Set *PREF to the decl or
>>expression REF refers to. */
>> @@ -167,7 +167,7 @@ trailing_array (tree arg, tree *pref)
>> return false;
>> }
>>
>> - return array_at_struct_end_p (arg);
>> + return array_ref_flexible_size_p (arg);
>> }
>>
>> /* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible
>> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
>> index 9055cd8982d..cafd331ca98 100644
>> --- a/gcc/gimple-fold.cc
>> +++ b/gcc/gimple-fold.cc
>> @@ -1690,13 +1690,11 @@ get_range_strlen_tree (tree arg, bitmap visited,
>> strlen_range_kind rkind,
>>/* Handle a MEM_REF into a DECL accessing an array of integers,
>> being conservative about references to extern structures with
>> flexible array members that can be initialized to arbitrary
>> - numbers of elements as an extension (static structs are okay).
>> - FIXME: Make this less conservative -- see
>> - component_ref_size in tree.cc. */
>> + numbers of elements as an extension (static structs are okay). */
>>tree ref = TREE_OPERAND (TREE_OPERAND (arg, 0), 0);
>>if ((TREE_CODE (ref) == PARM_DECL || VAR_P (ref))
>>&& (decl_binds_to_current_def_p (ref)
>> - || !array_at_struct_end_p (arg)))
>> + || !array_ref_flexible_size_p (arg)))
>> {
>>/* Fail if the offset is out of bounds. Such accesses
>> should be diagnosed at some point. */
>> diff --git
Re: [PATCH 1/2] Change the name of array_at_struct_end_p to array_ref_flexible_size_p
On Tue, 8 Nov 2022, Qing Zhao wrote:
> The name of the utility routine "array_at_struct_end_p" is misleading
> and should be changed to a new name that more accurately reflects its
> real meaning.
>
> The routine "array_at_struct_end_p" is used to check whether an array
> reference is to an array whose actual size might be larger than its
> upper bound implies, which includes 3 different cases:
>
>A. a ref to a flexible array member at the end of a structure;
>B. a ref to an array with a different type against the original decl;
>C. a ref to an array that was passed as a parameter;
>
> The old name only reflects the above case A, therefore very confusing
> when reading the corresponding gcc source code.
>
> In this patch, A new name "array_ref_flexible_size_p" is used to replace
> the old name.
>
> All the references to the routine "array_at_struct_end_p" was replaced
> with this new name, and the corresponding comments were updated to make
> them clean and consistent.
Since you seem to feel strongly about this - OK.
Thanks,
Richard.
> gcc/ChangeLog:
>
> * gimple-array-bounds.cc (trailing_array): Replace
> array_at_struct_end_p with new name and update comments.
> * gimple-fold.cc (get_range_strlen_tree): Likewise.
> * gimple-ssa-warn-restrict.cc (builtin_memref::builtin_memref):
> Likewise.
> * graphite-sese-to-poly.cc (bounds_are_valid): Likewise.
> * tree-if-conv.cc (idx_within_array_bound): Likewise.
> * tree-object-size.cc (addr_object_size): Likewise.
> * tree-ssa-alias.cc (component_ref_to_zero_sized_trailing_array_p):
> Likewise.
> (stmt_kills_ref_p): Likewise.
> * tree-ssa-loop-niter.cc (idx_infer_loop_bounds): Likewise.
> * tree-ssa-strlen.cc (maybe_set_strlen_range): Likewise.
> * tree.cc (array_at_struct_end_p): Rename to ...
> (array_ref_flexible_size_p): ... this.
> (component_ref_size): Replace array_at_struct_end_p with new name.
> * tree.h (array_at_struct_end_p): Rename to ...
> (array_ref_flexible_size_p): ... this.
> ---
> gcc/gimple-array-bounds.cc | 4 ++--
> gcc/gimple-fold.cc | 6 ++
> gcc/gimple-ssa-warn-restrict.cc | 5 +++--
> gcc/graphite-sese-to-poly.cc| 4 ++--
> gcc/tree-if-conv.cc | 7 +++
> gcc/tree-object-size.cc | 2 +-
> gcc/tree-ssa-alias.cc | 8
> gcc/tree-ssa-loop-niter.cc | 15 +++
> gcc/tree-ssa-strlen.cc | 2 +-
> gcc/tree.cc | 11 ++-
> gcc/tree.h | 8
> 11 files changed, 35 insertions(+), 37 deletions(-)
>
> diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
> index e190b93aa85..fbf448e045d 100644
> --- a/gcc/gimple-array-bounds.cc
> +++ b/gcc/gimple-array-bounds.cc
> @@ -129,7 +129,7 @@ get_ref_size (tree arg, tree *pref)
> }
>
> /* Return true if REF is (likely) an ARRAY_REF to a trailing array member
> - of a struct. It refines array_at_struct_end_p by detecting a pointer
> + of a struct. It refines array_ref_flexible_size_p by detecting a pointer
> to an array and an array parameter declared using the [N] syntax (as
> opposed to a pointer) and returning false. Set *PREF to the decl or
> expression REF refers to. */
> @@ -167,7 +167,7 @@ trailing_array (tree arg, tree *pref)
> return false;
> }
>
> - return array_at_struct_end_p (arg);
> + return array_ref_flexible_size_p (arg);
> }
>
> /* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index 9055cd8982d..cafd331ca98 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -1690,13 +1690,11 @@ get_range_strlen_tree (tree arg, bitmap visited,
> strlen_range_kind rkind,
> /* Handle a MEM_REF into a DECL accessing an array of integers,
>being conservative about references to extern structures with
>flexible array members that can be initialized to arbitrary
> - numbers of elements as an extension (static structs are okay).
> - FIXME: Make this less conservative -- see
> - component_ref_size in tree.cc. */
> + numbers of elements as an extension (static structs are okay). */
> tree ref = TREE_OPERAND (TREE_OPERAND (arg, 0), 0);
> if ((TREE_CODE (ref) == PARM_DECL || VAR_P (ref))
> && (decl_binds_to_current_def_p (ref)
> - || !array_at_struct_end_p (arg)))
> + || !array_ref_flexible_size_p (arg)))
> {
> /* Fail if the offset is out of bounds. Such accesses
>should be diagnosed at some point. */
> diff --git a/gcc/gimple-ssa-warn-restrict.cc b/gcc/gimple-ssa-warn-restrict.cc
> index b7ed15c8902..832456ba6fc 100644
> --- a/gcc/gimple-ssa-warn-restrict.cc
> +++ b/gcc/gimple-ssa-warn-restrict.cc
> @@ -296,8 +296,9 @@
[PATCH 1/2] Change the name of array_at_struct_end_p to array_ref_flexible_size_p
The name of the utility routine "array_at_struct_end_p" is misleading
and should be changed to a new name that more accurately reflects its
real meaning.
The routine "array_at_struct_end_p" is used to check whether an array
reference is to an array whose actual size might be larger than its
upper bound implies, which includes 3 different cases:
A. a ref to a flexible array member at the end of a structure;
B. a ref to an array with a different type against the original decl;
C. a ref to an array that was passed as a parameter;
The old name only reflects the above case A, therefore very confusing
when reading the corresponding gcc source code.
In this patch, A new name "array_ref_flexible_size_p" is used to replace
the old name.
All the references to the routine "array_at_struct_end_p" was replaced
with this new name, and the corresponding comments were updated to make
them clean and consistent.
gcc/ChangeLog:
* gimple-array-bounds.cc (trailing_array): Replace
array_at_struct_end_p with new name and update comments.
* gimple-fold.cc (get_range_strlen_tree): Likewise.
* gimple-ssa-warn-restrict.cc (builtin_memref::builtin_memref):
Likewise.
* graphite-sese-to-poly.cc (bounds_are_valid): Likewise.
* tree-if-conv.cc (idx_within_array_bound): Likewise.
* tree-object-size.cc (addr_object_size): Likewise.
* tree-ssa-alias.cc (component_ref_to_zero_sized_trailing_array_p):
Likewise.
(stmt_kills_ref_p): Likewise.
* tree-ssa-loop-niter.cc (idx_infer_loop_bounds): Likewise.
* tree-ssa-strlen.cc (maybe_set_strlen_range): Likewise.
* tree.cc (array_at_struct_end_p): Rename to ...
(array_ref_flexible_size_p): ... this.
(component_ref_size): Replace array_at_struct_end_p with new name.
* tree.h (array_at_struct_end_p): Rename to ...
(array_ref_flexible_size_p): ... this.
---
gcc/gimple-array-bounds.cc | 4 ++--
gcc/gimple-fold.cc | 6 ++
gcc/gimple-ssa-warn-restrict.cc | 5 +++--
gcc/graphite-sese-to-poly.cc| 4 ++--
gcc/tree-if-conv.cc | 7 +++
gcc/tree-object-size.cc | 2 +-
gcc/tree-ssa-alias.cc | 8
gcc/tree-ssa-loop-niter.cc | 15 +++
gcc/tree-ssa-strlen.cc | 2 +-
gcc/tree.cc | 11 ++-
gcc/tree.h | 8
11 files changed, 35 insertions(+), 37 deletions(-)
diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
index e190b93aa85..fbf448e045d 100644
--- a/gcc/gimple-array-bounds.cc
+++ b/gcc/gimple-array-bounds.cc
@@ -129,7 +129,7 @@ get_ref_size (tree arg, tree *pref)
}
/* Return true if REF is (likely) an ARRAY_REF to a trailing array member
- of a struct. It refines array_at_struct_end_p by detecting a pointer
+ of a struct. It refines array_ref_flexible_size_p by detecting a pointer
to an array and an array parameter declared using the [N] syntax (as
opposed to a pointer) and returning false. Set *PREF to the decl or
expression REF refers to. */
@@ -167,7 +167,7 @@ trailing_array (tree arg, tree *pref)
return false;
}
- return array_at_struct_end_p (arg);
+ return array_ref_flexible_size_p (arg);
}
/* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible
diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index 9055cd8982d..cafd331ca98 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -1690,13 +1690,11 @@ get_range_strlen_tree (tree arg, bitmap visited,
strlen_range_kind rkind,
/* Handle a MEM_REF into a DECL accessing an array of integers,
being conservative about references to extern structures with
flexible array members that can be initialized to arbitrary
-numbers of elements as an extension (static structs are okay).
-FIXME: Make this less conservative -- see
-component_ref_size in tree.cc. */
+numbers of elements as an extension (static structs are okay). */
tree ref = TREE_OPERAND (TREE_OPERAND (arg, 0), 0);
if ((TREE_CODE (ref) == PARM_DECL || VAR_P (ref))
&& (decl_binds_to_current_def_p (ref)
- || !array_at_struct_end_p (arg)))
+ || !array_ref_flexible_size_p (arg)))
{
/* Fail if the offset is out of bounds. Such accesses
should be diagnosed at some point. */
diff --git a/gcc/gimple-ssa-warn-restrict.cc b/gcc/gimple-ssa-warn-restrict.cc
index b7ed15c8902..832456ba6fc 100644
--- a/gcc/gimple-ssa-warn-restrict.cc
+++ b/gcc/gimple-ssa-warn-restrict.cc
@@ -296,8 +296,9 @@ builtin_memref::builtin_memref (pointer_query ,
gimple *stmt, tree expr,
tree basetype = TREE_TYPE (base);
if (TREE_CODE (basetype) == ARRAY_TYPE)
{
- if (ref && array_at_struct_end_p (ref))
- ; /* Use the maximum possible
