* 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).