* Re: Fix type of SRAed enum accesses
2011-09-27 15:36 Fix type of SRAed enum accesses Richard Sandiford
@ 2011-09-27 16:17 ` Richard Guenther
2011-09-27 16:44 ` Martin Jambor
2011-10-18 12:30 ` [4.6] " Jakub Jelinek
2 siblings, 0 replies; 5+ messages in thread
From: Richard Guenther @ 2011-09-27 16:17 UTC (permalink / raw)
To: gcc-patches, Martin Jambor, richard.sandiford
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Fix type of SRAed enum accesses
2011-09-27 15:36 Fix type of SRAed enum accesses Richard Sandiford
2011-09-27 16:17 ` Richard Guenther
@ 2011-09-27 16:44 ` Martin Jambor
2011-10-18 12:30 ` [4.6] " Jakub Jelinek
2 siblings, 0 replies; 5+ messages in thread
From: Martin Jambor @ 2011-09-27 16:44 UTC (permalink / raw)
To: gcc-patches, richard.sandiford
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [4.6] Fix type of SRAed enum accesses
2011-09-27 15:36 Fix type of SRAed enum accesses Richard Sandiford
2011-09-27 16:17 ` Richard Guenther
2011-09-27 16:44 ` Martin Jambor
@ 2011-10-18 12:30 ` Jakub Jelinek
2011-10-19 8:43 ` Richard Guenther
2 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2011-10-18 12:30 UTC (permalink / raw)
To: gcc-patches, Martin Jambor, richard.sandiford
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [4.6] Fix type of SRAed enum accesses
2011-10-18 12:30 ` [4.6] " Jakub Jelinek
@ 2011-10-19 8:43 ` Richard Guenther
0 siblings, 0 replies; 5+ messages in thread
From: Richard Guenther @ 2011-10-19 8:43 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches, Martin Jambor, richard.sandiford
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread