Re: [PATCH v2, middle-end]: Introduce TARGET_REJECT_COMBINED_INSN target hook

[Georg-Johann Lay <gjl at gcc dot gnu dot org>]

Ian Lance Taylor schrieb:
> On Sat, Aug 25, 2012 at 11:21 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> > On Sat, Aug 25, 2012 at 7:22 PM, Hans-Peter Nilsson <hp@bitrange.com> wrote:
> >
> > > > v2 patch differences:
> > > > - moves hook description text to target.def
> > > > - fixes error path to clear clobbers, as expected by recog_for_combine callers
> > > >
> > > > 2012-08-25  Uros Bizjak  <ubizjak@gmail.com>
> > > >
> > > >       * target.def (reject_combined_insn): New target hook.
> > > >       * doc/tm.texi.in (TARGET_REJECT_COMBINED_INSN): New hook.
> > > >       * doc/tm.texi: Regenerated.
> > > >       * combine.c (recog_for_combine): Call targetm.reject_combined_insn
> > > >       to allow targets to reject combined insn.
> > > Maybe mention that the default is to allow all combinations for
> > > which a pattern match?  And that the reason to disallow them can
> > > be that they're known to result in suboptimal code?  Or other
> > > reasons.
> > Something like this perhaps:
> >
> > /* Returns true if the combined insn should be rejected
> >    for some reason.  */
> > DEFHOOK
> > (reject_combined_insn,
> > "Take an instruction in @var{insn} and return @code{true} if the instruction\
> >  should be rejected as a combination of two or more instructions.  The\
> >  default is to accept all instructions.",
> >  bool, (rtx insn),
> >  hook_bool_rtx_false)
> >
> > > I don't think it should be mentioned that it can be used to stop
> > > invalid code to be generated; that'd just be encouraging
> > > covering up bugs in reload.  (Using it to simplify the port may
> > > be valid.)
> > Please note that there is no mention of the specific purpose of this
> > hook. x86 uses it to counter a specific reload problem, but this is
> > just hook _implementation_ detail.
>
>
> What if we just add a global variable during_combine, and the insn
> predicates can check that variable?  Then we wouldn't need a new
> target hook, one that seems slightly obscure to me.

Instead of a global variable, more information on passes would be a
good thing.

Currently there is reload_completed or reload_in_progress, but
sometimes similar information is needed for other passes like
IRA, split1 or combine.

A reasonable interface would be

compiler->passes->combine->finished
compiler->passes->split1->in_progress

or similar, but currently such a thing. The only way is by comparing
pass numbers; not nice.

Johann


> My apologies if this was already mentioned, I haven't followed the
> thread closely.
>
> Ian