* PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
@ 2011-04-05 0:51 H.J. Lu
2011-04-05 6:44 ` Paolo Bonzini
0 siblings, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2011-04-05 0:51 UTC (permalink / raw)
To: Michael Matz; +Cc: Richard Guenther, Paolo Bonzini, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1976 bytes --]
On Mon, Apr 4, 2011 at 3:00 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Apr 3, 2011 at 3:14 AM, Michael Matz <matz@suse.de> wrote:
>> Hi,
>>
>> On Thu, 31 Mar 2011, Richard Guenther wrote:
>>
>>> > In the meanwhile, is the below version okay?
>>>
>>> If it bootstraps & tests ok then yes. The java parts look obvious.
>>
>> So, we indeed can't remove the other calls to
>> canonicalize_constructor_val, because of local ctors. And fortran has a
>> similar problem with java. Instead of fixing up all these places of
>> resetting cfun (where otherwise the frontends don't deal at all with it,
>> it's mostly just set from the various cgraph routines), I decided to
>> simply clear this at the appropriate place in
>> cgraph_finalize_compilation_unit.
>>
>> Regstrapping in progress again. Still okay if that works?
>>
>>
>> Ciao,
>> Michael.
>> --
>> * cgraphbuild.c (record_reference): Canonicalize constructor
>> values.
>
>>
>> Index: cgraphbuild.c
>> ===================================================================
>> --- cgraphbuild.c.orig 2011-04-03 11:28:45.000000000 +0200
>> +++ cgraphbuild.c 2011-04-03 11:31:21.000000000 +0200
>> @@ -53,6 +53,12 @@ record_reference (tree *tp, int *walk_su
>> tree decl;
>> struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;
>>
>> + t = canonicalize_constructor_val (t);
>> + if (!t)
>> + t = *tp;
>> + else if (t != *tp)
>> + *tp = t;
>> +
>> switch (TREE_CODE (t))
>> {
>> case VAR_DECL:
>
> This change caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48440
>
This avoids canonicalizing constructor values for address conversion
if Pmode != ptr_mode. OK for trunk?
Thanks.
--
H.J.
----
2011-04-04 H.J. Lu <hongjiu.lu@intel.com>
PR middle-end/48440
* cgraphbuild.c (record_reference): Don't canonicalize constructor
values for address conversion if Pmode != ptr_mode.
[-- Attachment #2: gcc-pr48440-1.patch --]
[-- Type: text/plain, Size: 930 bytes --]
2011-04-04 H.J. Lu <hongjiu.lu@intel.com>
PR middle-end/48440
* cgraphbuild.c (record_reference): Don't canonicalize constructor
values for address conversion if Pmode != ptr_mode.
diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
index 3948cf6..8ba7864 100644
--- a/gcc/cgraphbuild.c
+++ b/gcc/cgraphbuild.c
@@ -49,15 +49,20 @@ struct record_reference_ctx
static tree
record_reference (tree *tp, int *walk_subtrees, void *data)
{
- tree t = *tp;
+ tree t = *tp, tc;
tree decl;
struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;
- t = canonicalize_constructor_val (t);
- if (!t)
- t = *tp;
- else if (t != *tp)
- *tp = t;
+ tc = canonicalize_constructor_val (t);
+ if (tc
+ && tc != t
+ && !(Pmode != ptr_mode
+ && TREE_CODE (tc) == ADDR_EXPR
+ && TREE_CODE (t) == CONVERT_EXPR))
+ {
+ *tp = tc;
+ t = tc;
+ }
switch (TREE_CODE (t))
{
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
2011-04-05 0:51 PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c H.J. Lu
@ 2011-04-05 6:44 ` Paolo Bonzini
2011-04-05 10:30 ` Richard Guenther
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2011-04-05 6:44 UTC (permalink / raw)
To: H.J. Lu; +Cc: Michael Matz, Richard Guenther, gcc-patches
>>> Index: cgraphbuild.c
>>> ===================================================================
>>> --- cgraphbuild.c.orig 2011-04-03 11:28:45.000000000 +0200
>>> +++ cgraphbuild.c 2011-04-03 11:31:21.000000000 +0200
>>> @@ -53,6 +53,12 @@ record_reference (tree *tp, int *walk_su
>>> tree decl;
>>> struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;
>>>
>>> + t = canonicalize_constructor_val (t);
>>> + if (!t)
>>> + t = *tp;
>>> + else if (t != *tp)
>>> + *tp = t;
>>> +
>>> switch (TREE_CODE (t))
>>> {
>>> case VAR_DECL:
>>
>> This change caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48440
>>
>
> This avoids canonicalizing constructor values for address conversion
> if Pmode != ptr_mode. OK for trunk?
Certainly the right place to fix it is in canonicalize_constructor_val itself.
Paolo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
2011-04-05 6:44 ` Paolo Bonzini
@ 2011-04-05 10:30 ` Richard Guenther
2011-04-06 17:03 ` H.J. Lu
0 siblings, 1 reply; 11+ messages in thread
From: Richard Guenther @ 2011-04-05 10:30 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: H.J. Lu, Michael Matz, gcc-patches
On Tue, Apr 5, 2011 at 8:44 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>> Index: cgraphbuild.c
>>>> ===================================================================
>>>> --- cgraphbuild.c.orig 2011-04-03 11:28:45.000000000 +0200
>>>> +++ cgraphbuild.c 2011-04-03 11:31:21.000000000 +0200
>>>> @@ -53,6 +53,12 @@ record_reference (tree *tp, int *walk_su
>>>> tree decl;
>>>> struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;
>>>>
>>>> + t = canonicalize_constructor_val (t);
>>>> + if (!t)
>>>> + t = *tp;
>>>> + else if (t != *tp)
>>>> + *tp = t;
>>>> +
>>>> switch (TREE_CODE (t))
>>>> {
>>>> case VAR_DECL:
>>>
>>> This change caused:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48440
>>>
>>
>> This avoids canonicalizing constructor values for address conversion
>> if Pmode != ptr_mode. OK for trunk?
>
> Certainly the right place to fix it is in canonicalize_constructor_val itself.
There should never be any Pmode pointer types in the gimple IL. The
proper place to fix it if any is in useless_type_conversion_p.
Richard.
> Paolo
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
2011-04-05 10:30 ` Richard Guenther
@ 2011-04-06 17:03 ` H.J. Lu
2011-04-07 8:51 ` Richard Guenther
0 siblings, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2011-04-06 17:03 UTC (permalink / raw)
To: Richard Guenther; +Cc: Paolo Bonzini, Michael Matz, gcc-patches
On Tue, Apr 5, 2011 at 3:29 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Tue, Apr 5, 2011 at 8:44 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>>> Index: cgraphbuild.c
>>>>> ===================================================================
>>>>> --- cgraphbuild.c.orig 2011-04-03 11:28:45.000000000 +0200
>>>>> +++ cgraphbuild.c 2011-04-03 11:31:21.000000000 +0200
>>>>> @@ -53,6 +53,12 @@ record_reference (tree *tp, int *walk_su
>>>>> tree decl;
>>>>> struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;
>>>>>
>>>>> + t = canonicalize_constructor_val (t);
>>>>> + if (!t)
>>>>> + t = *tp;
>>>>> + else if (t != *tp)
>>>>> + *tp = t;
>>>>> +
>>>>> switch (TREE_CODE (t))
>>>>> {
>>>>> case VAR_DECL:
>>>>
>>>> This change caused:
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48440
>>>>
>>>
>>> This avoids canonicalizing constructor values for address conversion
>>> if Pmode != ptr_mode. OK for trunk?
>>
>> Certainly the right place to fix it is in canonicalize_constructor_val itself.
>
> There should never be any Pmode pointer types in the gimple IL. The
> proper place to fix it if any is in useless_type_conversion_p.
>
We have
5600 newx = simplify_subreg (outermode, op, innermode, byte);
(gdb) f 1
#1 0x0000000000708494 in expand_expr_real_2 (ops=0x7fffffffb0c0, target=0x0,
tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
7366 op0 = simplify_gen_subreg (mode, op0, inner_mode,
(gdb) call debug_tree (treeop0)
<addr_expr 0x7ffff0a78d50
type <pointer_type 0x7ffff0b83f18
type <void_type 0x7ffff0b83e70 void VOID
align 8 symtab 0 alias set -1 canonical type 0x7ffff0b83e70
pointer_to_this <pointer_type 0x7ffff0b83f18>>
sizes-gimplified public unsigned SI
size <integer_cst 0x7ffff0b706e0 constant 32>
unit size <integer_cst 0x7ffff0b703e8 constant 4>
align 32 symtab 0 alias set -1 canonical type 0x7ffff0b83f18
pointer_to_this <pointer_type 0x7ffff0b985e8>>
constant
arg 0 <label_decl 0x7ffff0b7b400 l2 type <void_type 0x7ffff0b83e70 void>
side-effects VOID file x.i line 8 col 2
align 1 context <function_decl 0x7ffff0a68f00 foo> initial
<error_mark 0x7ffff0b789f0>
(code_label/s 22 0 0 4 4 ("l2") [2 uses])
chain <var_decl 0x7ffff0a790a0 p type <pointer_type 0x7ffff0b83f18>
used unsigned SI file x.i line 4 col 9 size <integer_cst
0x7ffff0b706e0 32> unit size <integer_cst 0x7ffff0b703e8 4>
align 32 context <function_decl 0x7ffff0a68f00 foo>
(mem/f/c/i:SI (plus:DI (reg/f:DI 20 frame)
(const_int -4 [0xfffffffffffffffc])) [0 p+0 S4 A32])>>
x.i:3:44>
(gdb) call debug_rtx (op0)
(label_ref/v:DI 22)
(gdb)
Since ptr_mode != Pmode, the type of op0 != type of treeop0.
Should we use GET_MODE (op0) instead of TYPE_MODE (inner_type)
here? Does this patch make any senses?
--
H.J.
---
diff --git a/gcc/expr.c b/gcc/expr.c
index d521f64..439f245 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -7360,7 +7360,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_m
ode tmode,
else if (CONSTANT_P (op0))
{
tree inner_type = TREE_TYPE (treeop0);
- enum machine_mode inner_mode = TYPE_MODE (inner_type);
+ enum machine_mode inner_mode = GET_MODE (op0);
if (modifier == EXPAND_INITIALIZER)
op0 = simplify_gen_subreg (mode, op0, inner_mode,
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
2011-04-06 17:03 ` H.J. Lu
@ 2011-04-07 8:51 ` Richard Guenther
2011-04-07 12:35 ` Michael Matz
0 siblings, 1 reply; 11+ messages in thread
From: Richard Guenther @ 2011-04-07 8:51 UTC (permalink / raw)
To: H.J. Lu; +Cc: Paolo Bonzini, Michael Matz, gcc-patches
On Wed, Apr 6, 2011 at 7:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Apr 5, 2011 at 3:29 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Tue, Apr 5, 2011 at 8:44 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>>>>>> Index: cgraphbuild.c
>>>>>> ===================================================================
>>>>>> --- cgraphbuild.c.orig 2011-04-03 11:28:45.000000000 +0200
>>>>>> +++ cgraphbuild.c 2011-04-03 11:31:21.000000000 +0200
>>>>>> @@ -53,6 +53,12 @@ record_reference (tree *tp, int *walk_su
>>>>>> tree decl;
>>>>>> struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;
>>>>>>
>>>>>> + t = canonicalize_constructor_val (t);
>>>>>> + if (!t)
>>>>>> + t = *tp;
>>>>>> + else if (t != *tp)
>>>>>> + *tp = t;
>>>>>> +
>>>>>> switch (TREE_CODE (t))
>>>>>> {
>>>>>> case VAR_DECL:
>>>>>
>>>>> This change caused:
>>>>>
>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48440
>>>>>
>>>>
>>>> This avoids canonicalizing constructor values for address conversion
>>>> if Pmode != ptr_mode. OK for trunk?
>>>
>>> Certainly the right place to fix it is in canonicalize_constructor_val itself.
>>
>> There should never be any Pmode pointer types in the gimple IL. The
>> proper place to fix it if any is in useless_type_conversion_p.
>>
>
> We have
>
> 5600 newx = simplify_subreg (outermode, op, innermode, byte);
> (gdb) f 1
> #1 0x0000000000708494 in expand_expr_real_2 (ops=0x7fffffffb0c0, target=0x0,
> tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
> at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
> 7366 op0 = simplify_gen_subreg (mode, op0, inner_mode,
> (gdb) call debug_tree (treeop0)
> <addr_expr 0x7ffff0a78d50
> type <pointer_type 0x7ffff0b83f18
> type <void_type 0x7ffff0b83e70 void VOID
> align 8 symtab 0 alias set -1 canonical type 0x7ffff0b83e70
> pointer_to_this <pointer_type 0x7ffff0b83f18>>
> sizes-gimplified public unsigned SI
> size <integer_cst 0x7ffff0b706e0 constant 32>
> unit size <integer_cst 0x7ffff0b703e8 constant 4>
> align 32 symtab 0 alias set -1 canonical type 0x7ffff0b83f18
> pointer_to_this <pointer_type 0x7ffff0b985e8>>
> constant
> arg 0 <label_decl 0x7ffff0b7b400 l2 type <void_type 0x7ffff0b83e70 void>
> side-effects VOID file x.i line 8 col 2
> align 1 context <function_decl 0x7ffff0a68f00 foo> initial
> <error_mark 0x7ffff0b789f0>
> (code_label/s 22 0 0 4 4 ("l2") [2 uses])
>
> chain <var_decl 0x7ffff0a790a0 p type <pointer_type 0x7ffff0b83f18>
> used unsigned SI file x.i line 4 col 9 size <integer_cst
> 0x7ffff0b706e0 32> unit size <integer_cst 0x7ffff0b703e8 4>
> align 32 context <function_decl 0x7ffff0a68f00 foo>
> (mem/f/c/i:SI (plus:DI (reg/f:DI 20 frame)
> (const_int -4 [0xfffffffffffffffc])) [0 p+0 S4 A32])>>
> x.i:3:44>
> (gdb) call debug_rtx (op0)
> (label_ref/v:DI 22)
> (gdb)
>
> Since ptr_mode != Pmode, the type of op0 != type of treeop0.
> Should we use GET_MODE (op0) instead of TYPE_MODE (inner_type)
> here? Does this patch make any senses?
First I wonder what CONSTANT_P object we arrive with here (it looks like
something unfolded, given that we likely came here with a NOP_EXPR).
Second, no, it doesn't make sense as CONST_INTs are modeless
(which is exactly why we look at tree types here ...)
The above gdb session paste is from a totally different place, so I can't
match the data to the place you are trying to patch.
Richard.
>
> --
> H.J.
> ---
> diff --git a/gcc/expr.c b/gcc/expr.c
> index d521f64..439f245 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -7360,7 +7360,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_m
> ode tmode,
> else if (CONSTANT_P (op0))
> {
> tree inner_type = TREE_TYPE (treeop0);
> - enum machine_mode inner_mode = TYPE_MODE (inner_type);
> + enum machine_mode inner_mode = GET_MODE (op0);
>
> if (modifier == EXPAND_INITIALIZER)
> op0 = simplify_gen_subreg (mode, op0, inner_mode,
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
2011-04-07 8:51 ` Richard Guenther
@ 2011-04-07 12:35 ` Michael Matz
2011-04-07 14:05 ` H.J. Lu
0 siblings, 1 reply; 11+ messages in thread
From: Michael Matz @ 2011-04-07 12:35 UTC (permalink / raw)
To: Richard Guenther; +Cc: H.J. Lu, Paolo Bonzini, gcc-patches
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1611 bytes --]
Hi,
On Thu, 7 Apr 2011, Richard Guenther wrote:
> > 5600 Â Â Â newx = simplify_subreg (outermode, op, innermode, byte);
> > (gdb) f 1
> > #1 Â 0x0000000000708494 in expand_expr_real_2 (ops=0x7fffffffb0c0, target=0x0,
> > Â Â tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
> > Â Â at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
> > 7366 Â Â Â Â Â Â Â Â op0 = simplify_gen_subreg (mode, op0, inner_mode,
> > (gdb) call debug_tree (treeop0)
> > Â <addr_expr 0x7ffff0a78d50
> > Â Â type <pointer_type 0x7ffff0b83f18
> > Â Â Â Â type <void_type 0x7ffff0b83e70 void VOID
> > Â Â Â Â Â Â align 8 symtab 0 alias set -1 canonical type 0x7ffff0b83e70
> > Â Â Â Â Â Â pointer_to_this <pointer_type 0x7ffff0b83f18>>
> > Â Â Â Â sizes-gimplified public unsigned SI
> > Â Â arg 0 <label_decl 0x7ffff0b7b400 l2 type <void_type 0x7ffff0b83e70 void>
> > (gdb) call debug_rtx (op0)
> > (label_ref/v:DI 22)
> > (gdb)
> >
>
> First I wonder what CONSTANT_P object we arrive with here (it looks like
> something unfolded, given that we likely came here with a NOP_EXPR).
The CONSTANT_P object is the '(label_ref/v:DI 22)'. Not only CONST_INT
are CONSTANT_P, also some others (symbol_ref too). Those might have a
mode. Here the label_ref has DImode, but the pointer type in trees points
to an SImode. The latter makes sense, because that's what ptr_mode is for
HJs target.
HJ: you'll need to tell us what you mean with 'breaks
gcc.c-torture/compile/labels-3.c' . What breaks, in which way?
Ciao,
Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
2011-04-07 12:35 ` Michael Matz
@ 2011-04-07 14:05 ` H.J. Lu
2011-04-15 23:34 ` Steve Ellcey
0 siblings, 1 reply; 11+ messages in thread
From: H.J. Lu @ 2011-04-07 14:05 UTC (permalink / raw)
To: Michael Matz; +Cc: Richard Guenther, Paolo Bonzini, gcc-patches
On Thu, Apr 7, 2011 at 5:34 AM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Thu, 7 Apr 2011, Richard Guenther wrote:
>
>> > 5600 newx = simplify_subreg (outermode, op, innermode, byte);
>> > (gdb) f 1
>> > #1 0x0000000000708494 in expand_expr_real_2 (ops=0x7fffffffb0c0, target=0x0,
>> > tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
>> > at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
>> > 7366 op0 = simplify_gen_subreg (mode, op0, inner_mode,
>> > (gdb) call debug_tree (treeop0)
>> > <addr_expr 0x7ffff0a78d50
>> > type <pointer_type 0x7ffff0b83f18
>> > type <void_type 0x7ffff0b83e70 void VOID
>> > align 8 symtab 0 alias set -1 canonical type 0x7ffff0b83e70
>> > pointer_to_this <pointer_type 0x7ffff0b83f18>>
>> > sizes-gimplified public unsigned SI
>> > arg 0 <label_decl 0x7ffff0b7b400 l2 type <void_type 0x7ffff0b83e70 void>
>> > (gdb) call debug_rtx (op0)
>> > (label_ref/v:DI 22)
>> > (gdb)
>> >
>>
>> First I wonder what CONSTANT_P object we arrive with here (it looks like
>> something unfolded, given that we likely came here with a NOP_EXPR).
>
> The CONSTANT_P object is the '(label_ref/v:DI 22)'. Not only CONST_INT
> are CONSTANT_P, also some others (symbol_ref too). Those might have a
> mode. Here the label_ref has DImode, but the pointer type in trees points
> to an SImode. The latter makes sense, because that's what ptr_mode is for
> HJs target.
>
> HJ: you'll need to tell us what you mean with 'breaks
> gcc.c-torture/compile/labels-3.c' . What breaks, in which way?
>
I got
#0 fancy_abort (
file=0x12470e0 "/export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c",
line=5266, function=0x1248470 "simplify_subreg")
at /export/gnu/import/git/gcc-x32/gcc/diagnostic.c:893
#1 0x00000000009d0ab5 in simplify_subreg (outermode=HImode, op=0x7ffff0c55dd0,
innermode=SImode, byte=0)
at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:5265
#2 0x00000000009d1e83 in simplify_gen_subreg (outermode=HImode,
op=0x7ffff0c55dd0, innermode=SImode, byte=0)
at /export/gnu/import/git/gcc-x32/gcc/simplify-rtx.c:5600
#3 0x0000000000708736 in expand_expr_real_2 (ops=0x7fffffffb480, target=0x0,
tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
#4 0x00000000007153e1 in expand_expr_real_1 (exp=0x7ffff0a87f30, target=0x0,
tmode=VOIDmode, modifier=EXPAND_INITIALIZER, alt_rtl=0x0)
at /export/gnu/import/git/gcc-x32/gcc/expr.c:9728
..
(gdb) f 3
#3 0x0000000000708736 in expand_expr_real_2 (ops=0x7fffffffb480, target=0x0,
tmode=VOIDmode, modifier=EXPAND_INITIALIZER)
at /export/gnu/import/git/gcc-x32/gcc/expr.c:7366
7366 op0 = simplify_gen_subreg (mode, op0, inner_mode,
(gdb) call debug_rtx (op0)
(label_ref/v:DI 22)
(gdb) p inner_mode
$2 = SImode
(gdb)
We are calling simplify_gen_subreg with wrong inner_mode.
--
H.J.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
2011-04-07 14:05 ` H.J. Lu
@ 2011-04-15 23:34 ` Steve Ellcey
2011-04-16 0:59 ` Michael Matz
0 siblings, 1 reply; 11+ messages in thread
From: Steve Ellcey @ 2011-04-15 23:34 UTC (permalink / raw)
To: H.J. Lu, Michael Matz, Richard Guenther, Paolo Bonzini; +Cc: gcc-patches
I was curious if anyone was still looking at this problem? I see this
on IA64 HP-UX in 32 bit mode where ptr_mode(SImode) != Pmode(DImode).
As H.J. points out, expand_expr_real_2 is calling simplify_gen_subreg
(expr.c, line 7366) and at that point op0 is "(label_ref/v:DI 32)" and
innermode is SImode. Because innermode doesn't match the mode of op0,
we abort in simplify_subreg. So it seems we either need to change how
we calculate innermode (so that it is DImode) or change expand_expr so
that it returns a SImode RTX expression for the tree treeop0. I am not
sure which of these is the right thing to do. The tree (treeop0) that
op0 is generated from is below and when we call expr_expr the modifier
is EXPAND_INITIALIZER. Should expand_expr return an SImode expression
for this tree (ptr_mode) or a DImode expression (Pmode) in this case?
<addr_expr 65866560
type <pointer_type 657d8960
type <void_type 657d8900 void VOID
align 8 symtab 0 alias set -1 canonical type 657d8900
pointer_to_this <pointer_type 657d8960>>
sizes-gimplified public unsigned SI
size <integer_cst 657d1920 constant 32>
unit size <integer_cst 657d16c0 constant 4>
align 32 symtab 0 alias set -1 canonical type 657d8960
pointer_to_this <pointer_type 657f1720> reference_to_this <reference_type 657f14e0>>
constant
arg 0 <label_decl 657e21b8 l2 type <void_type 657d8900 void>
side-effects VOID file labels-3.c line 16 col 2
align 1 context <function_decl 65858180 foo> initial <error_mark 657e6040>
(code_label/s 32 0 0 4 4 ("l2") [2 uses])
chain <var_decl 6585e3c0 p type <pointer_type 657d8960>
used unsigned SI file labels-3.c line 12 col 9 size <integer_cst 657d1920 32> unit size <integer_cst 657d16c0 4>
align 32 context <function_decl 65858180 foo>
(mem/f/c/i:SI (reg/f:DI 328 sfp) [0 p+0 S4 A32])>>
labels-3.c:11:44>
Steve Ellcey
sje@cup.hp.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
2011-04-15 23:34 ` Steve Ellcey
@ 2011-04-16 0:59 ` Michael Matz
2011-04-18 18:23 ` Steve Ellcey
0 siblings, 1 reply; 11+ messages in thread
From: Michael Matz @ 2011-04-16 0:59 UTC (permalink / raw)
To: Steve Ellcey; +Cc: H.J. Lu, Richard Guenther, Paolo Bonzini, gcc-patches
Hi Steve,
On Fri, 15 Apr 2011, Steve Ellcey wrote:
> I was curious if anyone was still looking at this problem?
I didn't because it occurred only on an experimental port.
> I see this on IA64 HP-UX in 32 bit mode
Which means it also occurs with something else now (well, ia64 hp-ux, but
at least something :) )
> to do. The tree (treeop0) that op0 is generated from is below and when
> we call expr_expr the modifier is EXPAND_INITIALIZER. Should
> expand_expr return an SImode expression for this tree (ptr_mode) or a
> DImode expression (Pmode) in this case?
Callers of expand_expr are expected to deal with the situation that the
returned object doesn't have the desired mode, hence I think it's okay for
expand_expr to return a DImode code_label rtx. Meaning we have to deal
with it. The patch of HJ is a first step but doesn't cater for op0 being
a CONST_INT, which doesn't have a mode, hence at the very least the code
should look like:
enum machine_mode inner_mode = GET_MODE (op0);
if (inner_mode == VOIDmode)
inner_mode = TYPE_MODE (inner_type);
I do wonder what other places in expand really would need something
similar. Right now nothing else ICEs but perhaps silently generates code
that only works by accident. Well, I guess we'll see.
Ciao,
Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
2011-04-16 0:59 ` Michael Matz
@ 2011-04-18 18:23 ` Steve Ellcey
2011-04-18 21:27 ` Richard Guenther
0 siblings, 1 reply; 11+ messages in thread
From: Steve Ellcey @ 2011-04-18 18:23 UTC (permalink / raw)
To: Michael Matz; +Cc: H.J. Lu, Richard Guenther, Paolo Bonzini, gcc-patches
On Sat, 2011-04-16 at 00:49 +0200, Michael Matz wrote:
> Callers of expand_expr are expected to deal with the situation that the
> returned object doesn't have the desired mode, hence I think it's okay for
> expand_expr to return a DImode code_label rtx. Meaning we have to deal
> with it. The patch of HJ is a first step but doesn't cater for op0 being
> a CONST_INT, which doesn't have a mode, hence at the very least the code
> should look like:
>
> enum machine_mode inner_mode = GET_MODE (op0);
> if (inner_mode == VOIDmode)
> inner_mode = TYPE_MODE (inner_type);
>
> I do wonder what other places in expand really would need something
> similar. Right now nothing else ICEs but perhaps silently generates code
> that only works by accident. Well, I guess we'll see.
>
>
> Ciao,
> Michael.
I tested your patch and it fixed the problem with labels-3.c. It also
ran through the test suite with no regressions on IA64 HP-UX and Linux
and on x86 Linux. I do see other places in the compiler where we call
GET_MODE and then check for VOIDmode to do something special/different
if we got that return value (in cfgexpand.c for example) so maybe this
is just one more place where we need this type of check.
Steve Ellcey
sje@cup.hp.com
Can someone approve this patch?
2011-05-18 Michael Matz <matz@suse.de>
Steve Ellcey <sje@cup.hp.com>
* expr.c (expand_expr_real_2): Use GET_MODE instead of TYPE_MODE.
Index: expr.c
===================================================================
--- expr.c (revision 172632)
+++ expr.c (working copy)
@@ -7360,8 +7360,11 @@
else if (CONSTANT_P (op0))
{
tree inner_type = TREE_TYPE (treeop0);
- enum machine_mode inner_mode = TYPE_MODE (inner_type);
+ enum machine_mode inner_mode = GET_MODE (op0);
+ if (inner_mode == VOIDmode)
+ inner_mode = TYPE_MODE (inner_type);
+
if (modifier == EXPAND_INITIALIZER)
op0 = simplify_gen_subreg (mode, op0, inner_mode,
subreg_lowpart_offset (mode,
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c
2011-04-18 18:23 ` Steve Ellcey
@ 2011-04-18 21:27 ` Richard Guenther
0 siblings, 0 replies; 11+ messages in thread
From: Richard Guenther @ 2011-04-18 21:27 UTC (permalink / raw)
To: sje; +Cc: Michael Matz, H.J. Lu, Paolo Bonzini, gcc-patches
On Mon, Apr 18, 2011 at 7:52 PM, Steve Ellcey <sje@cup.hp.com> wrote:
> On Sat, 2011-04-16 at 00:49 +0200, Michael Matz wrote:
>
>> Callers of expand_expr are expected to deal with the situation that the
>> returned object doesn't have the desired mode, hence I think it's okay for
>> expand_expr to return a DImode code_label rtx. Meaning we have to deal
>> with it. The patch of HJ is a first step but doesn't cater for op0 being
>> a CONST_INT, which doesn't have a mode, hence at the very least the code
>> should look like:
>>
>> enum machine_mode inner_mode = GET_MODE (op0);
>> if (inner_mode == VOIDmode)
>> inner_mode = TYPE_MODE (inner_type);
>>
>> I do wonder what other places in expand really would need something
>> similar. Right now nothing else ICEs but perhaps silently generates code
>> that only works by accident. Well, I guess we'll see.
>>
>>
>> Ciao,
>> Michael.
>
> I tested your patch and it fixed the problem with labels-3.c. It also
> ran through the test suite with no regressions on IA64 HP-UX and Linux
> and on x86 Linux. I do see other places in the compiler where we call
> GET_MODE and then check for VOIDmode to do something special/different
> if we got that return value (in cfgexpand.c for example) so maybe this
> is just one more place where we need this type of check.
>
> Steve Ellcey
> sje@cup.hp.com
>
> Can someone approve this patch?
Ok.
Thanks,
Richard.
>
>
> 2011-05-18 Michael Matz <matz@suse.de>
> Steve Ellcey <sje@cup.hp.com>
>
> * expr.c (expand_expr_real_2): Use GET_MODE instead of TYPE_MODE.
>
>
> Index: expr.c
> ===================================================================
> --- expr.c (revision 172632)
> +++ expr.c (working copy)
> @@ -7360,8 +7360,11 @@
> else if (CONSTANT_P (op0))
> {
> tree inner_type = TREE_TYPE (treeop0);
> - enum machine_mode inner_mode = TYPE_MODE (inner_type);
> + enum machine_mode inner_mode = GET_MODE (op0);
>
> + if (inner_mode == VOIDmode)
> + inner_mode = TYPE_MODE (inner_type);
> +
> if (modifier == EXPAND_INITIALIZER)
> op0 = simplify_gen_subreg (mode, op0, inner_mode,
> subreg_lowpart_offset (mode,
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-04-18 21:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-05 0:51 PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c H.J. Lu
2011-04-05 6:44 ` Paolo Bonzini
2011-04-05 10:30 ` Richard Guenther
2011-04-06 17:03 ` H.J. Lu
2011-04-07 8:51 ` Richard Guenther
2011-04-07 12:35 ` Michael Matz
2011-04-07 14:05 ` H.J. Lu
2011-04-15 23:34 ` Steve Ellcey
2011-04-16 0:59 ` Michael Matz
2011-04-18 18:23 ` Steve Ellcey
2011-04-18 21:27 ` Richard Guenther
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).