[2/4,og11] Unify ARRAY_REF/INDIRECT_REF stripping code in extract_base_bit_offset

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).

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(-)

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
>

On Wed, 2 Jun 2021 13:59:05 +0200
Richard Biener <richard.guenther@gmail.com> wrote:

> 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.

Thanks -- I'll go back and look at that bit again. I found it hard to
figure out the intention behind these differences, since AFAIK the
meaning should be the same -- maybe Jakub could weigh in as the
original author of this bit (I believe)?

(I figured something like "we can take this shortcut because we know we
already did XYZ to this bit" -- this part was originally completely
open-coded, i.e. there were two copies of this code, and it's only not
open-coded now because I tried to factor out the common bits...)

Julian