| Message ID | BANLkTi=xJeLSgRtOABREk_UL0XsvD_FELQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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, >
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.
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.
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
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.
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)