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

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

[Richard Biener]

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?

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

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

[Martin Jambor]

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