On Mon, Aug 18, 2014 at 4:00 AM, Patrick Palka <patr...@parcs.ath.cx> wrote:
> Hi,
>
> The fix for PR38615 indirectly broke the promotion of const local arrays
> to static storage in many cases. The commit in question, r143570, made
> it so that only arrays that don't potentially escape from the scope in
> which they're defined (i.e. arrays for which TREE_ADDRESSABLE is 0) are
> candidates for the promotion to static storage.
>
> The problem is that both the C and C++ frontends contain ancient code
> (dating back to 1994) that sets the TREE_ADDRESSABLE bit on arrays
> indexed by non-constant or out-of-range indices. As a result, const
> arrays that are indexed by non-constant or out-of-range values are no
> longer candidates for promotion to static storage following the fix to
> PR38615, because their TREE_ADDRESSABLE bit will always be set.
> Consequently, array promotion is essentially broken for a large class of
> C and C++ code. E.g. this simple example is no longer a candidate for
> promotion:
>
> int
> foo (int i)
> {
> const int x[] = { 1, 2, 3 };
> return x[i]; /* non-constant index */
> }
>
> This patch removes the ancient code that is responsible for dubiously
> setting the TREE_ADDRESSABLE bit on arrays indexed by non-constant or
> out-of-range values. I don't think that this code is necessary or
> useful anymore. Bootstrapped and regtested on x86_64-unknown-linux-gnu,
> OK for trunk?
This looks good to me - indeed TREE_ADDRESSABLE on variable-indexed
things isn't necessary (and before that it was made sure to re-set it
before RTL expansion which required it, as update-address-taken would
have happily removed TREE_ADDRESSABLE anyway).
Thanks,
Richard.
> 2014-08-18 Patrick Palka <ppa...@gcc.gnu.org>
>
> * c-typeck.c (build_array_ref): Don't mark arrays indexed by
> non-constant or out-of-range values as addressable.
>
> 2014-08-18 Patrick Palka <ppa...@gcc.gnu.org>
>
> * typeck.c (build_array_ref): Don't mark arrays indexed by
> non-constant or out-of-range values as addressable.
>
> 2014-08-18 Patrick Palka <ppa...@gcc.gnu.org>
>
> * g++.dg/opt/static4.C: Check for static promotion.
> * g++.dg/opt/static7.C: New test.
> * gcc.dg/static1.c: New test.
> ---
> gcc/c/c-typeck.c | 23 -----------------------
> gcc/cp/typeck.c | 25 -------------------------
> gcc/testsuite/g++.dg/opt/static4.C | 4 ++++
> gcc/testsuite/g++.dg/opt/static7.C | 12 ++++++++++++
> gcc/testsuite/gcc.dg/static1.c | 12 ++++++++++++
> 5 files changed, 28 insertions(+), 48 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/opt/static7.C
> create mode 100644 gcc/testsuite/gcc.dg/static1.c
>
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index e6745be..022ff8d 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -2487,29 +2487,6 @@ build_array_ref (location_t loc, tree array, tree
> index)
> {
> tree rval, type;
>
> - /* An array that is indexed by a non-constant
> - cannot be stored in a register; we must be able to do
> - address arithmetic on its address.
> - Likewise an array of elements of variable size. */
> - if (TREE_CODE (index) != INTEGER_CST
> - || (COMPLETE_TYPE_P (TREE_TYPE (TREE_TYPE (array)))
> - && TREE_CODE (TYPE_SIZE (TREE_TYPE (TREE_TYPE (array)))) !=
> INTEGER_CST))
> - {
> - if (!c_mark_addressable (array))
> - return error_mark_node;
> - }
> - /* An array that is indexed by a constant value which is not within
> - the array bounds cannot be stored in a register either; because we
> - would get a crash in store_bit_field/extract_bit_field when trying
> - to access a non-existent part of the register. */
> - if (TREE_CODE (index) == INTEGER_CST
> - && TYPE_DOMAIN (TREE_TYPE (array))
> - && !int_fits_type_p (index, TYPE_DOMAIN (TREE_TYPE (array))))
> - {
> - if (!c_mark_addressable (array))
> - return error_mark_node;
> - }
> -
> if (pedantic || warn_c90_c99_compat)
> {
> tree foo = array;
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index fa3283d..6dd056d 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -3112,31 +3112,6 @@ cp_build_array_ref (location_t loc, tree array, tree
> idx,
> pointer arithmetic.) */
> idx = cp_perform_integral_promotions (idx, complain);
>
> - /* An array that is indexed by a non-constant
> - cannot be stored in a register; we must be able to do
> - address arithmetic on its address.
> - Likewise an array of elements of variable size. */
> - if (TREE_CODE (idx) != INTEGER_CST
> - || (COMPLETE_TYPE_P (TREE_TYPE (TREE_TYPE (array)))
> - && (TREE_CODE (TYPE_SIZE (TREE_TYPE (TREE_TYPE (array))))
> - != INTEGER_CST)))
> - {
> - if (!cxx_mark_addressable (array))
> - return error_mark_node;
> - }
> -
> - /* An array that is indexed by a constant value which is not within
> - the array bounds cannot be stored in a register either; because we
> - would get a crash in store_bit_field/extract_bit_field when trying
> - to access a non-existent part of the register. */
> - if (TREE_CODE (idx) == INTEGER_CST
> - && TYPE_DOMAIN (TREE_TYPE (array))
> - && ! int_fits_type_p (idx, TYPE_DOMAIN (TREE_TYPE (array))))
> - {
> - if (!cxx_mark_addressable (array))
> - return error_mark_node;
> - }
> -
> if (!lvalue_p (array))
> {
> if (complain & tf_error)
> diff --git a/gcc/testsuite/g++.dg/opt/static4.C
> b/gcc/testsuite/g++.dg/opt/static4.C
> index 87e11b0..bae22c9 100644
> --- a/gcc/testsuite/g++.dg/opt/static4.C
> +++ b/gcc/testsuite/g++.dg/opt/static4.C
> @@ -3,6 +3,7 @@
> // if they are promoted to static storage.
>
> // { dg-do compile }
> +// { dg-options "-fdump-tree-gimple" }
>
> int g(int i) {
> if (i<1) {
> @@ -13,3 +14,6 @@ int g(int i) {
> return x[i];
> }
> }
> +
> +// { dg-final { scan-tree-dump-times "static const int" 2 "gimple" } }
> +// { dg-final { cleanup-tree-dump "gimple" } }
> diff --git a/gcc/testsuite/g++.dg/opt/static7.C
> b/gcc/testsuite/g++.dg/opt/static7.C
> new file mode 100644
> index 0000000..2a9f678
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/opt/static7.C
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fdump-tree-gimple" } */
> +
> +int
> +foo (int x)
> +{
> + const int y[] = { 1, 2, 3 };
> + return y[x];
> +}
> +
> +/* { dg-final { scan-tree-dump "static const int" "gimple" } } */
> +/* { dg-final { cleanup-tree-dump "gimple" } } */
> diff --git a/gcc/testsuite/gcc.dg/static1.c b/gcc/testsuite/gcc.dg/static1.c
> new file mode 100644
> index 0000000..2a9f678
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/static1.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fdump-tree-gimple" } */
> +
> +int
> +foo (int x)
> +{
> + const int y[] = { 1, 2, 3 };
> + return y[x];
> +}
> +
> +/* { dg-final { scan-tree-dump "static const int" "gimple" } } */
> +/* { dg-final { cleanup-tree-dump "gimple" } } */
> --
> 2.1.0
>
