Richard Biener <richard.guent...@gmail.com> writes:
> On Wed, Nov 25, 2015 at 1:20 PM, Richard Sandiford
> <richard.sandif...@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
