Re: [PATCH] tree-optimization/113727 - bogus SRA with BIT_FIELD_REF

[Martin Jambor <mjambor@suse.cz>]

Hello,

On Tue, Mar 19 2024, Richard Biener wrote:
> When SRA analyzes BIT_FIELD_REFs it handles writes and not byte
> aligned reads differently from byte aligned reads.  Instead of
> trying to create replacements for the loaded portion the former
> cases try to replace the base object while keeping the wrapping
> BIT_FIELD_REFs.  This breaks when we have both kinds operating
> on the same base object if there's no appearant overlap conflict
> as the conflict that then nevertheless exists isn't handled with.
> The fix is to enforce what I think is part of the design handling
> the former case - that only the full base object gets replaced
> and no further sub-objects are created within as otherwise
> keeping the wrapping BIT_FIELD_REF cannot work.  The patch
> enforces this within analyze_access_subtree.
>
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>
> OK?

I agree this is the best thing to do.

Thanks,

Martin

>
> Thanks,
> Richard.
>
> 	PR tree-optimization/113727
> 	* tree-sra.cc (analyze_access_subtree): Do not allow
> 	replacements in subtrees when grp_partial_lhs.
>
> 	* gcc.dg/torture/pr113727.c: New testcase.
> ---
>  gcc/testsuite/gcc.dg/torture/pr113727.c | 26 +++++++++++++++++++++++++
>  gcc/tree-sra.cc                         |  3 ++-
>  2 files changed, 28 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr113727.c
>
> diff --git a/gcc/testsuite/gcc.dg/torture/pr113727.c b/gcc/testsuite/gcc.dg/torture/pr113727.c
> new file mode 100644
> index 00000000000..f92ddad5c8e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr113727.c
> @@ -0,0 +1,26 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target int32plus } */
> +
> +struct f {
> +  unsigned au : 5;
> +  unsigned f3 : 21;
> +} g_994;
> +
> +int main()
> +{
> +  struct f aq1 = {};
> +    {
> +      struct f aq = {9, 5};
> +      struct f as = aq;
> +      for (int y = 0 ; y <= 4; y += 1)
> +	if (as.au)
> +	  {
> +	    struct f aa[5] = {{2, 154}, {2, 154}, {2, 154}, {2, 154}, {2, 154}};
> +	    as = aa[0];
> +	  }
> +      aq1 = as;
> +    }
> +  if (aq1.f3 != 0x9a)
> +    __builtin_abort();
> +  return 0;
> +}
> diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
> index f8e71ec48b9..dbfae5e7fdd 100644
> --- a/gcc/tree-sra.cc
> +++ b/gcc/tree-sra.cc
> @@ -2735,7 +2735,8 @@ analyze_access_subtree (struct access *root, struct access *parent,
>      {
>        hole |= covered_to < child->offset;
>        sth_created |= analyze_access_subtree (child, root,
> -					     allow_replacements && !scalar,
> +					     allow_replacements && !scalar
> +					     && !root->grp_partial_lhs,
>  					     totally);
>  
>        root->grp_unscalarized_data |= child->grp_unscalarized_data;
> -- 
> 2.35.3