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

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.

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