Fix type of SRAed enum accesses

Fix type of SRAed enum accesses

[Richard Sandiford]

This patch fixes a miscompilation of stage1 c-parser.o in an ARM bootstrap.
When an access to an enum field was SRAed, a component ref used the type
of the integer temporary variable instead of the type of the enum.
It therefore didn't alias other accesses to the same structure,
and was scheduled after a copy-load.

Tested on x86_64-linux-gnu, and by verifying that c-parser.o is correctly
compiled for ARM after the patch.  Martin says he's going to test on ia64
too (thanks) -- I'll add 50326 to the changelog if that goes OK.

OK to install if there are no regressions on ia64?

Richard


gcc/
	PR middle-end/50386
	* tree-sra.c (build_ref_for_model): Use the type of the field as
	the type of the COMPONENT_REF.

Index: gcc/tree-sra.c
===================================================================
--- gcc/tree-sra.c	2011-09-12 09:09:34.000000000 +0100
+++ gcc/tree-sra.c	2011-09-27 14:09:45.379960167 +0100
@@ -1504,7 +1504,7 @@ build_ref_for_model (location_t loc, tre
       offset -= TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (fld));
       exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0));
       t = build_ref_for_offset (loc, base, offset, exp_type, gsi, insert_after);
-      return fold_build3_loc (loc, COMPONENT_REF, model->type, t, fld,
+      return fold_build3_loc (loc, COMPONENT_REF, TREE_TYPE (fld), t, fld,
 			      TREE_OPERAND (model->expr, 2));
     }
   else

Re: Fix type of SRAed enum accesses

[Richard Guenther]

On Tue, Sep 27, 2011 at 4:26 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> This patch fixes a miscompilation of stage1 c-parser.o in an ARM bootstrap.
> When an access to an enum field was SRAed, a component ref used the type
> of the integer temporary variable instead of the type of the enum.
> It therefore didn't alias other accesses to the same structure,
> and was scheduled after a copy-load.
>
> Tested on x86_64-linux-gnu, and by verifying that c-parser.o is correctly
> compiled for ARM after the patch.  Martin says he's going to test on ia64
> too (thanks) -- I'll add 50326 to the changelog if that goes OK.
>
> OK to install if there are no regressions on ia64?

Ok.

Thanks,
Richard.

> Richard
>
>
> gcc/
>        PR middle-end/50386
>        * tree-sra.c (build_ref_for_model): Use the type of the field as
>        the type of the COMPONENT_REF.
>
> Index: gcc/tree-sra.c
> ===================================================================
> --- gcc/tree-sra.c      2011-09-12 09:09:34.000000000 +0100
> +++ gcc/tree-sra.c      2011-09-27 14:09:45.379960167 +0100
> @@ -1504,7 +1504,7 @@ build_ref_for_model (location_t loc, tre
>       offset -= TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (fld));
>       exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0));
>       t = build_ref_for_offset (loc, base, offset, exp_type, gsi, insert_after);
> -      return fold_build3_loc (loc, COMPONENT_REF, model->type, t, fld,
> +      return fold_build3_loc (loc, COMPONENT_REF, TREE_TYPE (fld), t, fld,
>                              TREE_OPERAND (model->expr, 2));
>     }
>   else
>

Re: Fix type of SRAed enum accesses

[Martin Jambor]

Hi,

On Tue, Sep 27, 2011 at 03:26:03PM +0100, Richard Sandiford wrote:
> This patch fixes a miscompilation of stage1 c-parser.o in an ARM bootstrap.
> When an access to an enum field was SRAed, a component ref used the type
> of the integer temporary variable instead of the type of the enum.
> It therefore didn't alias other accesses to the same structure,
> and was scheduled after a copy-load.
> 
> Tested on x86_64-linux-gnu, and by verifying that c-parser.o is correctly
> compiled for ARM after the patch.  Martin says he's going to test on ia64
> too (thanks) -- I'll add 50326 to the changelog if that goes OK.
> 
> OK to install if there are no regressions on ia64?
> 

C and C++ bootstrap passed fine on ia64 with this patch so it is
indeed the correct fix for PR 50326.  Thanks!

Martin


> Richard
> 
> 
> gcc/
> 	PR middle-end/50386
> 	* tree-sra.c (build_ref_for_model): Use the type of the field as
> 	the type of the COMPONENT_REF.
> 
> Index: gcc/tree-sra.c
> ===================================================================
> --- gcc/tree-sra.c	2011-09-12 09:09:34.000000000 +0100
> +++ gcc/tree-sra.c	2011-09-27 14:09:45.379960167 +0100
> @@ -1504,7 +1504,7 @@ build_ref_for_model (location_t loc, tre
>        offset -= TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (fld));
>        exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0));
>        t = build_ref_for_offset (loc, base, offset, exp_type, gsi, insert_after);
> -      return fold_build3_loc (loc, COMPONENT_REF, model->type, t, fld,
> +      return fold_build3_loc (loc, COMPONENT_REF, TREE_TYPE (fld), t, fld,
>  			      TREE_OPERAND (model->expr, 2));
>      }
>    else

[4.6] Fix type of SRAed enum accesses

[Jakub Jelinek]

On Tue, Sep 27, 2011 at 03:26:03PM +0100, Richard Sandiford wrote:
> This patch fixes a miscompilation of stage1 c-parser.o in an ARM bootstrap.
> When an access to an enum field was SRAed, a component ref used the type
> of the integer temporary variable instead of the type of the enum.
> It therefore didn't alias other accesses to the same structure,
> and was scheduled after a copy-load.
> 
> Tested on x86_64-linux-gnu, and by verifying that c-parser.o is correctly
> compiled for ARM after the patch.  Martin says he's going to test on ia64
> too (thanks) -- I'll add 50326 to the changelog if that goes OK.
> 
> OK to install if there are no regressions on ia64?

As mentioned in bugzilla, the patch that caused this regression has been
committed to 4.6 branch as well.

Here is a backport of that patch to 4.6 branch, bootstrapped/regtested on
x86_64-linux and i686-linux and Matthias AFAIK bootstrapped it on ia64-linux
(where it previously failed to bootstrap because of this), regtest pending.

Ok for 4.6?

2011-10-18  Jakub Jelinek  <jakub@redhat.com>

	PR target/50350
	Backport from mainline
	2011-09-27  Richard Sandiford  <rdsandiford@googlemail.com>

	PR middle-end/50386
	PR middle-end/50326
	* tree-sra.c (build_ref_for_model): Use the type of the field as
	the type of the COMPONENT_REF.

--- gcc/tree-sra.c.jj	2011-09-30 17:19:10.000000000 +0200
+++ gcc/tree-sra.c	2011-10-18 08:01:28.682647833 +0200
@@ -1448,12 +1448,12 @@ build_ref_for_model (location_t loc, tre
 {
   if (TREE_CODE (model->expr) == COMPONENT_REF)
     {
-      tree t, exp_type;
-      offset -= int_bit_position (TREE_OPERAND (model->expr, 1));
+      tree t, exp_type, fld = TREE_OPERAND (model->expr, 1);
+      offset -= int_bit_position (fld);
       exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0));
       t = build_ref_for_offset (loc, base, offset, exp_type, gsi, insert_after);
-      return fold_build3_loc (loc, COMPONENT_REF, model->type, t,
-			      TREE_OPERAND (model->expr, 1), NULL_TREE);
+      return fold_build3_loc (loc, COMPONENT_REF, TREE_TYPE (fld), t, fld,
+			      NULL_TREE);
     }
   else
     return build_ref_for_offset (loc, base, offset, model->type,

	Jakub

Re: [4.6] Fix type of SRAed enum accesses

[Richard Guenther]

On Tue, Oct 18, 2011 at 2:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Sep 27, 2011 at 03:26:03PM +0100, Richard Sandiford wrote:
>> This patch fixes a miscompilation of stage1 c-parser.o in an ARM bootstrap.
>> When an access to an enum field was SRAed, a component ref used the type
>> of the integer temporary variable instead of the type of the enum.
>> It therefore didn't alias other accesses to the same structure,
>> and was scheduled after a copy-load.
>>
>> Tested on x86_64-linux-gnu, and by verifying that c-parser.o is correctly
>> compiled for ARM after the patch.  Martin says he's going to test on ia64
>> too (thanks) -- I'll add 50326 to the changelog if that goes OK.
>>
>> OK to install if there are no regressions on ia64?
>
> As mentioned in bugzilla, the patch that caused this regression has been
> committed to 4.6 branch as well.
>
> Here is a backport of that patch to 4.6 branch, bootstrapped/regtested on
> x86_64-linux and i686-linux and Matthias AFAIK bootstrapped it on ia64-linux
> (where it previously failed to bootstrap because of this), regtest pending.
>
> Ok for 4.6?

Ok.

Richard.

> 2011-10-18  Jakub Jelinek  <jakub@redhat.com>
>
>        PR target/50350
>        Backport from mainline
>        2011-09-27  Richard Sandiford  <rdsandiford@googlemail.com>
>
>        PR middle-end/50386
>        PR middle-end/50326
>        * tree-sra.c (build_ref_for_model): Use the type of the field as
>        the type of the COMPONENT_REF.
>
> --- gcc/tree-sra.c.jj   2011-09-30 17:19:10.000000000 +0200
> +++ gcc/tree-sra.c      2011-10-18 08:01:28.682647833 +0200
> @@ -1448,12 +1448,12 @@ build_ref_for_model (location_t loc, tre
>  {
>   if (TREE_CODE (model->expr) == COMPONENT_REF)
>     {
> -      tree t, exp_type;
> -      offset -= int_bit_position (TREE_OPERAND (model->expr, 1));
> +      tree t, exp_type, fld = TREE_OPERAND (model->expr, 1);
> +      offset -= int_bit_position (fld);
>       exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0));
>       t = build_ref_for_offset (loc, base, offset, exp_type, gsi, insert_after);
> -      return fold_build3_loc (loc, COMPONENT_REF, model->type, t,
> -                             TREE_OPERAND (model->expr, 1), NULL_TREE);
> +      return fold_build3_loc (loc, COMPONENT_REF, TREE_TYPE (fld), t, fld,
> +                             NULL_TREE);
>     }
>   else
>     return build_ref_for_offset (loc, base, offset, model->type,
>
>        Jakub
>