RFA: Fix ICE on PARALLEL returns when expand builtins

[Richard Biener]

On Wed, Jan 2, 2013 at 5:36 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Sun, Dec 23, 2012 at 10:43 AM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> Some of the maths builtins can expand to a call followed by a bit
>>> of postprocessing.  With 4.8's PARALLEL return optimisations, these
>>> embedded calls might return a PARALLEL of pseudos, but the postprocessing
>>> isn't prepared to deal with that.  This leads to an ICE in builtins-53.c
>>> on n32 and n64 mips64-linux-gnu.
>>>
>>> One fix might have been to pass an explicit register target to the
>>> expand routines, but that target's only a hint.  This patch instead
>>> adds an avoid_group_rtx function (named after gen_group_rtx) to convert
>>> PARALLELs to pseudos where necessary.
>>>
>>> I wondered whether it was really safe for expand_builtin_int_roundingfn_2
>>> to pass "target == const0_rtx" as the "ignore" parameter to expand_call,
>>> given that we don't actually ignore the return value ourselves
>>> (even if the caller does).  I suppose it is safe though,
>>> since expand_call will always return const0_rtx in that case,
>>> and this code is dealing with integer return values.
>>>
>>> Tested on mips64-linux-gnu.  OK to install?  Or is there a better way?
>>
>> You didn't add a testcase so I can't check myself
>
> It's gcc.dg/builtins-53.c.
>
>> - but why isn't using force_reg enough here?  I can imagine other
>> cases than PARALLELs that are not well handled, no?
>
> Not sure either way TBH.  Fortunately expanding your own calls seems
> to be pretty rare.
>
> But yeah, having force_reg (or I suppose force_operand) do it sounds
> good in principle.  The problem is that the operation needs the type
> tree, which the force_* routines don't have.

Hm?  force_reg/operand only need a mode.  I'm suggesting sth like

Index: builtins.c
===================================================================
*** builtins.c  (revision 194787)
--- builtins.c  (working copy)
*************** expand_builtin_int_roundingfn (tree exp,
*** 2760,2765 ****
--- 2760,2766 ----

    /* Truncate the result of floating point optab to integer
       via expand_fix ().  */
+   tmp = force_reg (TYPE_MODE (TREE_TYPE (TREE_TYPE (fallback_fndecl))), tmp);
    target = gen_reg_rtx (mode);
    expand_fix (target, tmp, 0);

(I suppose we can even use force_reg (GET_MODE (tmp), tmp) here - it shouldn't
be VOIDmode ever as its of FP type).

Richard.

> Richard