Richard Biener <richard.guent...@gmail.com> writes: > On Wed, Jan 2, 2013 at 5:36 PM, Richard Sandiford > <rdsandif...@googlemail.com> wrote: >> Richard Biener <richard.guent...@gmail.com> writes: >>> On Sun, Dec 23, 2012 at 10:43 AM, Richard Sandiford >>> <rdsandif...@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. > > 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);
What I mean is: force_operand doesn't convert PARALLELs of pseudos to single pseudos, so this won't work as-is. And we can't make force_operand do the conversion using the current emit_group_store machinery because emit_group_store needs access to the type (in order to work out the padding), which force_operand doesn't have. Richard
