public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] tree-sra: Fix union handling in build_reconstructed_reference (PR 105860)
@ 2022-07-01 20:48 Martin Jambor
  2022-07-04  6:15 ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Jambor @ 2022-07-01 20:48 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Biener

Hi,

As the testcase in PR 105860 shows, the code that tries to re-use the
handled_component chains in SRA can be horribly confused by unions,
where it thinks it has found a compatible structure under which it can
chain the references, but in fact it found the type it was looking
for elsewhere in a union and generated a write to a completely wrong
part of an aggregate.

I don't remember whether the plan was to support unions at all in
build_reconstructed_reference but it can work, to an extent, if we
make sure that we start the search only outside the outermost union,
which is what the patch does (and the extra testcase verifies).

Bootstrapped and tested on x86_64-linux.  OK for trunk and then for
release branches?

Thanks,

Martin


gcc/ChangeLog:

2022-07-01  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/105860
	* tree-sra.cc (build_reconstructed_reference): Start expr
	traversal only just below the outermost union.

gcc/testsuite/ChangeLog:

2022-07-01  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/105860
	* gcc.dg/tree-ssa/alias-access-path-13.c: New test.
	* gcc.dg/tree-ssa/pr105860.c: Likewise.
---
 .../gcc.dg/tree-ssa/alias-access-path-13.c    | 31 +++++++++
 gcc/testsuite/gcc.dg/tree-ssa/pr105860.c      | 63 +++++++++++++++++++
 gcc/tree-sra.cc                               | 13 +++-
 3 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr105860.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
new file mode 100644
index 00000000000..e502a97bc75
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+
+struct inn
+{
+  int val;
+};
+
+union foo
+{
+  struct inn inn;
+  long int baz;
+} *fooptr;
+
+struct bar
+{
+  union foo foo;
+  int val2;
+} *barptr;
+
+int
+test ()
+{
+  union foo foo;
+  foo.inn.val = 0;
+  barptr->val2 = 123;
+  *fooptr = foo;
+  return barptr->val2;
+}
+
+/* { dg-final { scan-tree-dump-times "return 123" 1 "fre1"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c b/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
new file mode 100644
index 00000000000..77bcb4a6739
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
@@ -0,0 +1,63 @@
+/* { dg-do run } */
+/* { dg-options "-O1" } */
+
+struct S1  {
+        unsigned int _0;
+        unsigned int _1;
+} ;
+struct S2  {
+        struct S1 _s1;
+        unsigned long _x2;
+} ;
+
+struct ufld_type1  {
+        unsigned int _u1t;
+        struct S2 _s2;
+} ;
+
+struct ufld_type2  {
+        unsigned int _u2t;
+        struct S1 _s1;
+} ;
+struct parm_type {
+        union {
+                struct ufld_type1 var_1;
+                struct ufld_type2 var_2;
+        } U;
+};
+
+struct parm_type  bad_function( struct parm_type arg0 )
+{
+        struct parm_type rv;
+        struct S2 var4;
+        switch( arg0.U.var_2._u2t ) {
+        case 4294967041:
+                var4._s1 = arg0.U.var_1._s2._s1;
+                rv.U.var_1._u1t = 4294967041;
+                rv.U.var_1._s2 = var4;
+                break;
+        case 4294967043:
+                rv.U.var_2._u2t = 4294967043;
+                rv.U.var_2._s1 = arg0.U.var_2._s1;
+                break;
+        default:
+                break;
+        }
+        return rv;
+}
+
+int main() {
+        struct parm_type val;
+        struct parm_type out;
+        val.U.var_2._u2t = 4294967043;
+        val.U.var_2._s1._0 = 0x01010101;
+        val.U.var_2._s1._1 = 0x02020202;
+        out = bad_function(val);
+	if (val.U.var_2._u2t != 4294967043)
+	  __builtin_abort ();
+        if (out.U.var_2._s1._0 != 0x01010101)
+	  __builtin_abort ();
+        if (val.U.var_2._s1._1 != 0x02020202 )
+	  __builtin_abort ();
+	return 0;
+}
diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
index 081c51b58a4..099e8dbe873 100644
--- a/gcc/tree-sra.cc
+++ b/gcc/tree-sra.cc
@@ -1667,7 +1667,18 @@ build_ref_for_offset (location_t loc, tree base, poly_int64 offset,
 static tree
 build_reconstructed_reference (location_t, tree base, struct access *model)
 {
-  tree expr = model->expr, prev_expr = NULL;
+  tree expr = model->expr;
+  /* We have to make sure to start just below the outermost union.  */
+  tree start_expr = expr;
+  while (handled_component_p (expr))
+    {
+      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (expr, 0))) == UNION_TYPE)
+	start_expr = expr;
+      expr = TREE_OPERAND (expr, 0);
+    }
+
+  expr = start_expr;
+  tree prev_expr = NULL_TREE;
   while (!types_compatible_p (TREE_TYPE (expr), TREE_TYPE (base)))
     {
       if (!handled_component_p (expr))
-- 
2.36.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tree-sra: Fix union handling in build_reconstructed_reference (PR 105860)
  2022-07-01 20:48 [PATCH] tree-sra: Fix union handling in build_reconstructed_reference (PR 105860) Martin Jambor
@ 2022-07-04  6:15 ` Richard Biener
  2022-07-04 14:48   ` Martin Jambor
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2022-07-04  6:15 UTC (permalink / raw)
  To: Martin Jambor; +Cc: GCC Patches

On Fri, 1 Jul 2022, Martin Jambor wrote:

> Hi,
> 
> As the testcase in PR 105860 shows, the code that tries to re-use the
> handled_component chains in SRA can be horribly confused by unions,
> where it thinks it has found a compatible structure under which it can
> chain the references, but in fact it found the type it was looking
> for elsewhere in a union and generated a write to a completely wrong
> part of an aggregate.
> 
> I don't remember whether the plan was to support unions at all in
> build_reconstructed_reference but it can work, to an extent, if we
> make sure that we start the search only outside the outermost union,
> which is what the patch does (and the extra testcase verifies).
> 
> Bootstrapped and tested on x86_64-linux.  OK for trunk and then for
> release branches?

OK, but I'm wondering if the same problem can arise when the
handled_component_p includes VIEW_CONVERTs or BIT_FIELD_REFs
both of which may pun to a type already seen in a more inner
referece.  Thus, is the actual problem that build_reconstructed_reference
searches for the outermost match of the type rather than the
innermost match?  So should it search inner-to-outer instead
(or for the last match in the current way of searching?)

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> gcc/ChangeLog:
> 
> 2022-07-01  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR tree-optimization/105860
> 	* tree-sra.cc (build_reconstructed_reference): Start expr
> 	traversal only just below the outermost union.
> 
> gcc/testsuite/ChangeLog:
> 
> 2022-07-01  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR tree-optimization/105860
> 	* gcc.dg/tree-ssa/alias-access-path-13.c: New test.
> 	* gcc.dg/tree-ssa/pr105860.c: Likewise.
> ---
>  .../gcc.dg/tree-ssa/alias-access-path-13.c    | 31 +++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/pr105860.c      | 63 +++++++++++++++++++
>  gcc/tree-sra.cc                               | 13 +++-
>  3 files changed, 106 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
> 
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
> new file mode 100644
> index 00000000000..e502a97bc75
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
> @@ -0,0 +1,31 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-fre1" } */
> +
> +struct inn
> +{
> +  int val;
> +};
> +
> +union foo
> +{
> +  struct inn inn;
> +  long int baz;
> +} *fooptr;
> +
> +struct bar
> +{
> +  union foo foo;
> +  int val2;
> +} *barptr;
> +
> +int
> +test ()
> +{
> +  union foo foo;
> +  foo.inn.val = 0;
> +  barptr->val2 = 123;
> +  *fooptr = foo;
> +  return barptr->val2;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "return 123" 1 "fre1"} } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c b/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
> new file mode 100644
> index 00000000000..77bcb4a6739
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
> @@ -0,0 +1,63 @@
> +/* { dg-do run } */
> +/* { dg-options "-O1" } */
> +
> +struct S1  {
> +        unsigned int _0;
> +        unsigned int _1;
> +} ;
> +struct S2  {
> +        struct S1 _s1;
> +        unsigned long _x2;
> +} ;
> +
> +struct ufld_type1  {
> +        unsigned int _u1t;
> +        struct S2 _s2;
> +} ;
> +
> +struct ufld_type2  {
> +        unsigned int _u2t;
> +        struct S1 _s1;
> +} ;
> +struct parm_type {
> +        union {
> +                struct ufld_type1 var_1;
> +                struct ufld_type2 var_2;
> +        } U;
> +};
> +
> +struct parm_type  bad_function( struct parm_type arg0 )
> +{
> +        struct parm_type rv;
> +        struct S2 var4;
> +        switch( arg0.U.var_2._u2t ) {
> +        case 4294967041:
> +                var4._s1 = arg0.U.var_1._s2._s1;
> +                rv.U.var_1._u1t = 4294967041;
> +                rv.U.var_1._s2 = var4;
> +                break;
> +        case 4294967043:
> +                rv.U.var_2._u2t = 4294967043;
> +                rv.U.var_2._s1 = arg0.U.var_2._s1;
> +                break;
> +        default:
> +                break;
> +        }
> +        return rv;
> +}
> +
> +int main() {
> +        struct parm_type val;
> +        struct parm_type out;
> +        val.U.var_2._u2t = 4294967043;
> +        val.U.var_2._s1._0 = 0x01010101;
> +        val.U.var_2._s1._1 = 0x02020202;
> +        out = bad_function(val);
> +	if (val.U.var_2._u2t != 4294967043)
> +	  __builtin_abort ();
> +        if (out.U.var_2._s1._0 != 0x01010101)
> +	  __builtin_abort ();
> +        if (val.U.var_2._s1._1 != 0x02020202 )
> +	  __builtin_abort ();
> +	return 0;
> +}
> diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
> index 081c51b58a4..099e8dbe873 100644
> --- a/gcc/tree-sra.cc
> +++ b/gcc/tree-sra.cc
> @@ -1667,7 +1667,18 @@ build_ref_for_offset (location_t loc, tree base, poly_int64 offset,
>  static tree
>  build_reconstructed_reference (location_t, tree base, struct access *model)
>  {
> -  tree expr = model->expr, prev_expr = NULL;
> +  tree expr = model->expr;
> +  /* We have to make sure to start just below the outermost union.  */
> +  tree start_expr = expr;
> +  while (handled_component_p (expr))
> +    {
> +      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (expr, 0))) == UNION_TYPE)
> +	start_expr = expr;
> +      expr = TREE_OPERAND (expr, 0);
> +    }
> +
> +  expr = start_expr;
> +  tree prev_expr = NULL_TREE;
>    while (!types_compatible_p (TREE_TYPE (expr), TREE_TYPE (base)))
>      {
>        if (!handled_component_p (expr))
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstra

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tree-sra: Fix union handling in build_reconstructed_reference (PR 105860)
  2022-07-04  6:15 ` Richard Biener
@ 2022-07-04 14:48   ` Martin Jambor
  2022-07-05  7:34     ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Jambor @ 2022-07-04 14:48 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

Hi,

On Mon, Jul 04 2022, Richard Biener wrote:
> On Fri, 1 Jul 2022, Martin Jambor wrote:
>
>> Hi,
>> 
>> As the testcase in PR 105860 shows, the code that tries to re-use the
>> handled_component chains in SRA can be horribly confused by unions,
>> where it thinks it has found a compatible structure under which it can
>> chain the references, but in fact it found the type it was looking
>> for elsewhere in a union and generated a write to a completely wrong
>> part of an aggregate.
>> 
>> I don't remember whether the plan was to support unions at all in
>> build_reconstructed_reference but it can work, to an extent, if we
>> make sure that we start the search only outside the outermost union,
>> which is what the patch does (and the extra testcase verifies).
>> 
>> Bootstrapped and tested on x86_64-linux.  OK for trunk and then for
>> release branches?
>
> OK, but I'm wondering if the same problem can arise when the
> handled_component_p includes VIEW_CONVERTs or BIT_FIELD_REFs
> both of which may pun to a type already seen in a more inner
> referece.

SRA already refuses to operate at all on any anything that is accessed
with a reference where a V_C_E is not the outermost handled_component.
Outermost V_C_Es are skipped and the pass works with the underlying
expr.  BIT_FIELD_REFs have to be outermost and they are treated
similarly.  So that should be safe.

> Thus, is the actual problem that build_reconstructed_reference
> searches for the outermost match of the type rather than the
> innermost match?  So should it search inner-to-outer instead
> (or for the last match in the current way of searching?)

I don't think so.  In the testcase it found a match where there should
have been none (meaning a crude MEM_REF should be created), any
certainly correct match must be outside of a union COMPONENT_REF and
there should never be more than one.

Thanks,

Martin

>
> Thanks,
> Richard.
>
>> Thanks,
>> 
>> Martin
>> 
>> 
>> gcc/ChangeLog:
>> 
>> 2022-07-01  Martin Jambor  <mjambor@suse.cz>
>> 
>> 	PR tree-optimization/105860
>> 	* tree-sra.cc (build_reconstructed_reference): Start expr
>> 	traversal only just below the outermost union.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 2022-07-01  Martin Jambor  <mjambor@suse.cz>
>> 
>> 	PR tree-optimization/105860
>> 	* gcc.dg/tree-ssa/alias-access-path-13.c: New test.
>> 	* gcc.dg/tree-ssa/pr105860.c: Likewise.
>> ---
>>  .../gcc.dg/tree-ssa/alias-access-path-13.c    | 31 +++++++++
>>  gcc/testsuite/gcc.dg/tree-ssa/pr105860.c      | 63 +++++++++++++++++++
>>  gcc/tree-sra.cc                               | 13 +++-
>>  3 files changed, 106 insertions(+), 1 deletion(-)
>>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
>>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
>> 
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
>> new file mode 100644
>> index 00000000000..e502a97bc75
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
>> @@ -0,0 +1,31 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fdump-tree-fre1" } */
>> +
>> +struct inn
>> +{
>> +  int val;
>> +};
>> +
>> +union foo
>> +{
>> +  struct inn inn;
>> +  long int baz;
>> +} *fooptr;
>> +
>> +struct bar
>> +{
>> +  union foo foo;
>> +  int val2;
>> +} *barptr;
>> +
>> +int
>> +test ()
>> +{
>> +  union foo foo;
>> +  foo.inn.val = 0;
>> +  barptr->val2 = 123;
>> +  *fooptr = foo;
>> +  return barptr->val2;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump-times "return 123" 1 "fre1"} } */
>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c b/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
>> new file mode 100644
>> index 00000000000..77bcb4a6739
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
>> @@ -0,0 +1,63 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O1" } */
>> +
>> +struct S1  {
>> +        unsigned int _0;
>> +        unsigned int _1;
>> +} ;
>> +struct S2  {
>> +        struct S1 _s1;
>> +        unsigned long _x2;
>> +} ;
>> +
>> +struct ufld_type1  {
>> +        unsigned int _u1t;
>> +        struct S2 _s2;
>> +} ;
>> +
>> +struct ufld_type2  {
>> +        unsigned int _u2t;
>> +        struct S1 _s1;
>> +} ;
>> +struct parm_type {
>> +        union {
>> +                struct ufld_type1 var_1;
>> +                struct ufld_type2 var_2;
>> +        } U;
>> +};
>> +
>> +struct parm_type  bad_function( struct parm_type arg0 )
>> +{
>> +        struct parm_type rv;
>> +        struct S2 var4;
>> +        switch( arg0.U.var_2._u2t ) {
>> +        case 4294967041:
>> +                var4._s1 = arg0.U.var_1._s2._s1;
>> +                rv.U.var_1._u1t = 4294967041;
>> +                rv.U.var_1._s2 = var4;
>> +                break;
>> +        case 4294967043:
>> +                rv.U.var_2._u2t = 4294967043;
>> +                rv.U.var_2._s1 = arg0.U.var_2._s1;
>> +                break;
>> +        default:
>> +                break;
>> +        }
>> +        return rv;
>> +}
>> +
>> +int main() {
>> +        struct parm_type val;
>> +        struct parm_type out;
>> +        val.U.var_2._u2t = 4294967043;
>> +        val.U.var_2._s1._0 = 0x01010101;
>> +        val.U.var_2._s1._1 = 0x02020202;
>> +        out = bad_function(val);
>> +	if (val.U.var_2._u2t != 4294967043)
>> +	  __builtin_abort ();
>> +        if (out.U.var_2._s1._0 != 0x01010101)
>> +	  __builtin_abort ();
>> +        if (val.U.var_2._s1._1 != 0x02020202 )
>> +	  __builtin_abort ();
>> +	return 0;
>> +}
>> diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
>> index 081c51b58a4..099e8dbe873 100644
>> --- a/gcc/tree-sra.cc
>> +++ b/gcc/tree-sra.cc
>> @@ -1667,7 +1667,18 @@ build_ref_for_offset (location_t loc, tree base, poly_int64 offset,
>>  static tree
>>  build_reconstructed_reference (location_t, tree base, struct access *model)
>>  {
>> -  tree expr = model->expr, prev_expr = NULL;
>> +  tree expr = model->expr;
>> +  /* We have to make sure to start just below the outermost union.  */
>> +  tree start_expr = expr;
>> +  while (handled_component_p (expr))
>> +    {
>> +      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (expr, 0))) == UNION_TYPE)
>> +	start_expr = expr;
>> +      expr = TREE_OPERAND (expr, 0);
>> +    }
>> +
>> +  expr = start_expr;
>> +  tree prev_expr = NULL_TREE;
>>    while (!types_compatible_p (TREE_TYPE (expr), TREE_TYPE (base)))
>>      {
>>        if (!handled_component_p (expr))
>> 
>
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Frankenstra

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tree-sra: Fix union handling in build_reconstructed_reference (PR 105860)
  2022-07-04 14:48   ` Martin Jambor
@ 2022-07-05  7:34     ` Richard Biener
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2022-07-05  7:34 UTC (permalink / raw)
  To: Martin Jambor; +Cc: GCC Patches

On Mon, 4 Jul 2022, Martin Jambor wrote:

> Hi,
> 
> On Mon, Jul 04 2022, Richard Biener wrote:
> > On Fri, 1 Jul 2022, Martin Jambor wrote:
> >
> >> Hi,
> >> 
> >> As the testcase in PR 105860 shows, the code that tries to re-use the
> >> handled_component chains in SRA can be horribly confused by unions,
> >> where it thinks it has found a compatible structure under which it can
> >> chain the references, but in fact it found the type it was looking
> >> for elsewhere in a union and generated a write to a completely wrong
> >> part of an aggregate.
> >> 
> >> I don't remember whether the plan was to support unions at all in
> >> build_reconstructed_reference but it can work, to an extent, if we
> >> make sure that we start the search only outside the outermost union,
> >> which is what the patch does (and the extra testcase verifies).
> >> 
> >> Bootstrapped and tested on x86_64-linux.  OK for trunk and then for
> >> release branches?
> >
> > OK, but I'm wondering if the same problem can arise when the
> > handled_component_p includes VIEW_CONVERTs or BIT_FIELD_REFs
> > both of which may pun to a type already seen in a more inner
> > referece.
> 
> SRA already refuses to operate at all on any anything that is accessed
> with a reference where a V_C_E is not the outermost handled_component.
> Outermost V_C_Es are skipped and the pass works with the underlying
> expr.  BIT_FIELD_REFs have to be outermost and they are treated
> similarly.  So that should be safe.

OK.

> > Thus, is the actual problem that build_reconstructed_reference
> > searches for the outermost match of the type rather than the
> > innermost match?  So should it search inner-to-outer instead
> > (or for the last match in the current way of searching?)
> 
> I don't think so.  In the testcase it found a match where there should
> have been none (meaning a crude MEM_REF should be created), any
> certainly correct match must be outside of a union COMPONENT_REF and
> there should never be more than one.

OK, not having looked at the testcase I suspected there would be
two matches.  I couldn't come up with a case where a single union
can confuse things enough ..

The patch is OK.

Richard.

> Thanks,
> 
> Martin
> 
> >
> > Thanks,
> > Richard.
> >
> >> Thanks,
> >> 
> >> Martin
> >> 
> >> 
> >> gcc/ChangeLog:
> >> 
> >> 2022-07-01  Martin Jambor  <mjambor@suse.cz>
> >> 
> >> 	PR tree-optimization/105860
> >> 	* tree-sra.cc (build_reconstructed_reference): Start expr
> >> 	traversal only just below the outermost union.
> >> 
> >> gcc/testsuite/ChangeLog:
> >> 
> >> 2022-07-01  Martin Jambor  <mjambor@suse.cz>
> >> 
> >> 	PR tree-optimization/105860
> >> 	* gcc.dg/tree-ssa/alias-access-path-13.c: New test.
> >> 	* gcc.dg/tree-ssa/pr105860.c: Likewise.
> >> ---
> >>  .../gcc.dg/tree-ssa/alias-access-path-13.c    | 31 +++++++++
> >>  gcc/testsuite/gcc.dg/tree-ssa/pr105860.c      | 63 +++++++++++++++++++
> >>  gcc/tree-sra.cc                               | 13 +++-
> >>  3 files changed, 106 insertions(+), 1 deletion(-)
> >>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
> >>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
> >> 
> >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
> >> new file mode 100644
> >> index 00000000000..e502a97bc75
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-13.c
> >> @@ -0,0 +1,31 @@
> >> +/* { dg-do compile } */
> >> +/* { dg-options "-O2 -fdump-tree-fre1" } */
> >> +
> >> +struct inn
> >> +{
> >> +  int val;
> >> +};
> >> +
> >> +union foo
> >> +{
> >> +  struct inn inn;
> >> +  long int baz;
> >> +} *fooptr;
> >> +
> >> +struct bar
> >> +{
> >> +  union foo foo;
> >> +  int val2;
> >> +} *barptr;
> >> +
> >> +int
> >> +test ()
> >> +{
> >> +  union foo foo;
> >> +  foo.inn.val = 0;
> >> +  barptr->val2 = 123;
> >> +  *fooptr = foo;
> >> +  return barptr->val2;
> >> +}
> >> +
> >> +/* { dg-final { scan-tree-dump-times "return 123" 1 "fre1"} } */
> >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c b/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
> >> new file mode 100644
> >> index 00000000000..77bcb4a6739
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr105860.c
> >> @@ -0,0 +1,63 @@
> >> +/* { dg-do run } */
> >> +/* { dg-options "-O1" } */
> >> +
> >> +struct S1  {
> >> +        unsigned int _0;
> >> +        unsigned int _1;
> >> +} ;
> >> +struct S2  {
> >> +        struct S1 _s1;
> >> +        unsigned long _x2;
> >> +} ;
> >> +
> >> +struct ufld_type1  {
> >> +        unsigned int _u1t;
> >> +        struct S2 _s2;
> >> +} ;
> >> +
> >> +struct ufld_type2  {
> >> +        unsigned int _u2t;
> >> +        struct S1 _s1;
> >> +} ;
> >> +struct parm_type {
> >> +        union {
> >> +                struct ufld_type1 var_1;
> >> +                struct ufld_type2 var_2;
> >> +        } U;
> >> +};
> >> +
> >> +struct parm_type  bad_function( struct parm_type arg0 )
> >> +{
> >> +        struct parm_type rv;
> >> +        struct S2 var4;
> >> +        switch( arg0.U.var_2._u2t ) {
> >> +        case 4294967041:
> >> +                var4._s1 = arg0.U.var_1._s2._s1;
> >> +                rv.U.var_1._u1t = 4294967041;
> >> +                rv.U.var_1._s2 = var4;
> >> +                break;
> >> +        case 4294967043:
> >> +                rv.U.var_2._u2t = 4294967043;
> >> +                rv.U.var_2._s1 = arg0.U.var_2._s1;
> >> +                break;
> >> +        default:
> >> +                break;
> >> +        }
> >> +        return rv;
> >> +}
> >> +
> >> +int main() {
> >> +        struct parm_type val;
> >> +        struct parm_type out;
> >> +        val.U.var_2._u2t = 4294967043;
> >> +        val.U.var_2._s1._0 = 0x01010101;
> >> +        val.U.var_2._s1._1 = 0x02020202;
> >> +        out = bad_function(val);
> >> +	if (val.U.var_2._u2t != 4294967043)
> >> +	  __builtin_abort ();
> >> +        if (out.U.var_2._s1._0 != 0x01010101)
> >> +	  __builtin_abort ();
> >> +        if (val.U.var_2._s1._1 != 0x02020202 )
> >> +	  __builtin_abort ();
> >> +	return 0;
> >> +}
> >> diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
> >> index 081c51b58a4..099e8dbe873 100644
> >> --- a/gcc/tree-sra.cc
> >> +++ b/gcc/tree-sra.cc
> >> @@ -1667,7 +1667,18 @@ build_ref_for_offset (location_t loc, tree base, poly_int64 offset,
> >>  static tree
> >>  build_reconstructed_reference (location_t, tree base, struct access *model)
> >>  {
> >> -  tree expr = model->expr, prev_expr = NULL;
> >> +  tree expr = model->expr;
> >> +  /* We have to make sure to start just below the outermost union.  */
> >> +  tree start_expr = expr;
> >> +  while (handled_component_p (expr))
> >> +    {
> >> +      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (expr, 0))) == UNION_TYPE)
> >> +	start_expr = expr;
> >> +      expr = TREE_OPERAND (expr, 0);
> >> +    }
> >> +
> >> +  expr = start_expr;
> >> +  tree prev_expr = NULL_TREE;
> >>    while (!types_compatible_p (TREE_TYPE (expr), TREE_TYPE (base)))
> >>      {
> >>        if (!handled_component_p (expr))
> >> 
> >
> > -- 
> > Richard Biener <rguenther@suse.de>
> > SUSE Software Solutions Germany GmbH, Frankenstra
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstra

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-07-05  7:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-01 20:48 [PATCH] tree-sra: Fix union handling in build_reconstructed_reference (PR 105860) Martin Jambor
2022-07-04  6:15 ` Richard Biener
2022-07-04 14:48   ` Martin Jambor
2022-07-05  7:34     ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).