Bernd Schmidt <bschm...@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