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

[Richard Sandiford]

Bernd Schmidt <bschmidt@redhat.com> writes:
> On 11/26/2015 05:22 PM, Richard Sandiford wrote:
>> Bernd Schmidt <bschmidt@redhat.com> writes:
>>
>>> I wish we'd taken some more time to think through the consequences of
>>> the original internal_fn patchset.
>>
>> I don't think this PR shows that the approach was wrong.
>
> I think it does. Internal functions make a new assumptions, that 
> expanders don't FAIL - but as we've now seen, they do. The optimize_size 
> thing is reasonably easy to grep for and it looks like only i386 is 
> affected, but have you looked at every expander in every port that could 
> be used by an internal function to ensure it does not FAIL for a 
> different reason?

I've tried and I couldn't see any other problems.

I don't think what you say is an argument that the approach is wrong.
The C conditions for optabs have always been more restricted than
other define_expands and define_insns, since they cannot refer
to operands.  When caching of optabs was added, they also lost
the ability to test for size/speed choices.  There have also
always been optabs that are not allowed to FAIL (such as moves,
get_thread_pointer, widening multiplication, vec_cond, etc.).
This series is extending that list, but it's in the spirit
of restrictions that have always existed.  I don't see that
that's an argument that the approach is wrong.

The current approach to FAILs dated from a time when expand was
the first code-generation pass.  The FAILs aren't a good fit for
gimple optimisers that are trying to find out what the target
can do (and how cheaply it can do it).

> Is there a simple way to disable the entire internal_fn machinery and 
> get us back to where we were in gcc-5, without taking out all the code 
> immediately? That would give us time until next stage 1 to think through 
> the issues.

That seems like an overreaction.

I went for the 22-patch series because I think it's the best fix for
this problem.  It also makes the existing enabled, preferred_for_size
and preferred_for_speed handling more robust (as shown by the ARM bug
that the structural changes exposed at compile time).  But there are
other less-invasive ways of fixing it too, as described in the thread
about rsqrt.  I'm going to work on that today.

Thanks,
Richard