Re: PATCH: PR middle-end/48440: [4.7 Regression] FAIL: gcc.c-torture/compile/labels-3.c

[Richard Guenther <richard.guenther@gmail.com>]

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,
>