[PR68432 00/26] Handle size/speed choices for internal functions

[Richard Sandiford]

Richard Biener <richard.guenther@gmail.com> writes:
> On Wed, Nov 25, 2015 at 1:20 PM, Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>> This series fixes PR 68432, a regression caused by my internal-functions-
>> for-optabs series.  Some of the libm optabs in i386.md have a true HAVE_*
>> condition but conditionally FAIL if we're optimising for size:
>>
>>   if (SSE_FLOAT_MODE_P (<MODE>mode) && TARGET_SSE_MATH
>>       && !flag_trapping_math)
>>     {
>>       if (TARGET_ROUND)
>>         emit_insn (gen_sse4_1_round<mode>2
>>                    (operands[0], operands[1], GEN_INT (ROUND_MXCSR)));
>>       else if (optimize_insn_for_size_p ())
>>         FAIL;
>>       else
>>         ix86_expand_rint (operands[0], operands[1]);
>>     }
>>
>> This is going to cause problems if we want to make more decisions
>> related to optabs at the gimple level.
>>
>> We've already had the same problem in rtl, where some patterns used
>> to check optimize_insn_for_size_p in their C condition, and would then
>> fail to match if an instruction were moved from a hot block to a cold
>> block.  Now rtl has two attributes, preferred_for_size and
>> preferred_for_speed, that say whether a particular alternative of a
>> particular instruction should be used when optimising for size or speed
>> respectively.  We try to honour a "false" value as much as possible,
>> but it's not an absolute guarantee.
>>
>> The point of this series is to extend preferred_for_size and
>> preferred_for_speed to define_expands, so that the attributes
>> can be tested for optabs too.
>
> So to not re-introduce the same position issue at GIMPLE (passes
> querying optimize_bb_for_speed/size and with that querying
> the optab for IFN support) when expanding an internal function
> you ignore whether it actually "exists"?  That is, IFN expansion
> will skip the define_expand (which is maybe disabled)?

We ignore the size and speed attributes when expanding an existing
function call, yeah.  But to me "exists" means the C condition in the
define_expand or define_insn, which we check even when expanding.
(We effectively check it whenever direct_optab or convert_optab
is called, thanks to caching by init_optabs.)  We should never generate
an expand or insn if the C condition is false.

In other words, the size and speed attributes are additional information
on top of the "exists" condition that we check when deciding whether to
create a call, but not when expanding an existing call.

I forgot to say that the patch only handles optabs that are mapped
to internal functions.  I think in next stage 1 it would make sense to
do the same for all optabs, but that would be quite invasive and I don't
think it would fix a bug as such.

Thanks,
Richard