Re: [PATCH 2/4] [og11] Unify ARRAY_REF/INDIRECT_REF stripping code in extract_base_bit_offset

[Richard Biener <richard.guenther@gmail.com>]

On Wed, Jun 2, 2021 at 12:47 PM Julian Brown <julian@codesourcery.com> wrote:
>
> For historical reasons, it seems that extract_base_bit_offset
> unnecessarily used two different ways to strip ARRAY_REF/INDIRECT_REF
> nodes from component accesses. I verified that the two ways of performing
> the operation gave the same results across the whole testsuite (and
> several additional benchmarks).

But the two code paths clearly do sth different.  The base_ref case
allows (*p)[i] while the !base_ref does not because TREE_CODE (base)
!= COMPONENT_REF.
And the !base_ref case for INDIRECT_REF is quite odd, only allowing
*(x.p) where x.p is of REFERENCE_TYPE.

Whatever this code is supposed to do ... maybe the "prologue" should be inlined
at the two callers instead.

Richard.

> The code was like this since an earlier "mechanical" refactoring by me,
> first posted here:
>
>   https://gcc.gnu.org/pipermail/gcc-patches/2018-November/510503.html
>
> It was never clear to me if there was an important semantic
> difference between the two ways of stripping the base before calling
> get_inner_reference, but it appears that there is not, so one can go away.
>
> 2021-06-02  Julian Brown  <julian@codesourcery.com>
>
> gcc/
>         * gimplify.c (extract_base_bit_offset): Unify ARRAY_REF/INDIRECT_REF
>         stripping code in first call/subsequent call cases.
> ---
>  gcc/gimplify.c | 32 +++++++++++---------------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index a38cd502aa5..255a2a648c1 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -8527,31 +8527,21 @@ extract_base_bit_offset (tree base, tree *base_ref, poly_int64 *bitposp,
>    poly_offset_int poffset;
>
>    if (base_ref)
> -    {
> -      *base_ref = NULL_TREE;
> -
> -      while (TREE_CODE (base) == ARRAY_REF)
> -       base = TREE_OPERAND (base, 0);
> +    *base_ref = NULL_TREE;
>
> -      if (TREE_CODE (base) == INDIRECT_REF)
> -       base = TREE_OPERAND (base, 0);
> -    }
> -  else
> +  if (TREE_CODE (base) == ARRAY_REF)
>      {
> -      if (TREE_CODE (base) == ARRAY_REF)
> -       {
> -         while (TREE_CODE (base) == ARRAY_REF)
> -           base = TREE_OPERAND (base, 0);
> -         if (TREE_CODE (base) != COMPONENT_REF
> -             || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE)
> -           return NULL_TREE;
> -       }
> -      else if (TREE_CODE (base) == INDIRECT_REF
> -              && TREE_CODE (TREE_OPERAND (base, 0)) == COMPONENT_REF
> -              && (TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0)))
> -                  == REFERENCE_TYPE))
> +      while (TREE_CODE (base) == ARRAY_REF)
>         base = TREE_OPERAND (base, 0);
> +      if (TREE_CODE (base) != COMPONENT_REF
> +         || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE)
> +       return NULL_TREE;
>      }
> +  else if (TREE_CODE (base) == INDIRECT_REF
> +          && TREE_CODE (TREE_OPERAND (base, 0)) == COMPONENT_REF
> +          && (TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0)))
> +              == REFERENCE_TYPE))
> +    base = TREE_OPERAND (base, 0);
>
>    base = get_inner_reference (base, &bitsize, &bitpos, &offset, &mode,
>                               &unsignedp, &reversep, &volatilep);
> --
> 2.29.2
>