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

[Richard Sandiford]

Bernd Schmidt <bschmidt@redhat.com> writes:
> On 11/26/2015 04:13 PM, Richard Sandiford wrote:
>> That would mean that the validity of a gimple call would depend on both
>> the target predicates and whether the block containing the statement
>> is optimised for size or speed.  So whenever we want to test whether
>> a gimple call is valid, we'd need to generate rtl for its arguments
>> and pass them to the target predicates.  We'd also need to be aware
>> that moving a call between blocks could make it invalid (because
>> we might be moving a call from a block optimised for speed to a block
>> optimised for size).  I don't think those are the kinds of thing that
>> gimple passes would normally expect.
>
> In your world, would we move such calls into places where we currently 
> would reject expanding them? I.e., would the expanders no longer fail, 
> even if the target does not want to expand something when optimizing for 
> size?

Yeah, that could happen in theory, though should be rare in practice.
We already had this problem in rtl when patterns tried to test
optimize_insn_for_size/speed_p in their insn conditions.  See e.g.:

  https://gcc.gnu.org/ml/gcc-patches/2014-04/msg00697.html

> The other question is, you mention the need to generate rtl. I don't 
> quite see why - the predicates (or insn conditions, to follow the 
> terminology in the manual) for a named pattern aren't allowed to look at 
> operands. Surely these conditions are already taken into account in your 
> internal_fn work?

Ah, when you said "predicate" I assumed you meant the match_operand
predicates.  We'd need to generate rtl to check those.  But checking
size/speed in the insn condition doesn't work for the reason above: we
could move an instruction between blocks after deciding to generate it.

It also isn't suitable for optabs because the conditions are cached
by init_optabs.  I suppose we could have a separate cache for size
and speed though.

>> It seems better to use FAILs and predicates for correctness only
>> and use other ways of representing size/speed decisions.  And since we
>> already have another way for rtl, it seems like a good idea to use it
>> for gimple too.
>
> I'm looking for a minimal fix for gcc-6, and a 22-patch series that 
> rewrites lots of target patterns isn't that. If someone else wants to 
> approve it then fine, but I think this is not a good approach for now.

OK.  But I'd also like to avoid a hacky solution just because of the
stage.  Although we're in stage 3, we're still early stage 3,
so I'm hoping there's a bit of leeway.

> To avoid having to retest validity when moving an internal function, 
> could you just make the availability test run the predicate with both 
> for_speed and for_size options, and require that the pattern is valid 
> for both? That should give you a definitive answer as to whether you can 
> later expand the insn, and I'd call that good enough for now.

That would mean we'd never use rint for x86 before expand.

> 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.

Thanks,
Richard