Bernd Schmidt <bschm...@redhat.com> writes: > On 11/26/2015 05:22 PM, Richard Sandiford wrote: >> Bernd Schmidt <bschm...@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