Re: [PATCH v3 2/3] Add predict_doloop_p target hook

[Segher Boessenkool <segher@kernel.crashing.org>]

On Tue, May 21, 2019 at 01:50:31PM +0800, Kewen.Lin wrote:
> on 2019/5/20 下午10:43, Jeff Law wrote:
> > On 5/20/19 4:24 AM, Segher Boessenkool wrote:
> >> Let me try to answer a bit here...
> >>
> >> On Mon, May 20, 2019 at 11:28:26AM +0200, Richard Biener wrote:
> >>> On Thu, 16 May 2019, linkw@linux.ibm.com wrote: 
> >>
> >>> So the better way would be to expose that via a target hook somehow.
> >>> Or simply restrict IVOPTs processing to innermost loops for now.
> >>
> >> I think we should have two hooks: one is called with the struct loop as
> >> parameter; and the other is called for every statement in the loop, if
> >> the hook isn't null anyway.  Or perhaps we do not need that second one.
> > I'd wait to see a compelling example from real world code where we need
> > to scan the statements.  Otherwise we're just dragging in more target
> > specific decisions which in fact we want to minimize target stuff.
> 
> The scan is trying to do similar thing like default_invalid_within_doloop.
> It scans for hardware counter register clobbering.  I think it's important
> and valuable to scan especially for call since it's common.

Ah, right, without this check we would say many more loops can be doloop
than can in fact be.

>   if (CALL_P (insn))
>     return "Function call in loop.";
> 
>   if (tablejump_p (insn, NULL, NULL) || computed_jump_p (insn))
>     return "Computed branch in the loop.";
> 
> But it's a question whether to make it as part of generic.  I double checked
> that most of the doloop targets use this default behavior, only 5 targets are
> using their own TARGET_INVALID_WITHIN_DOLOOP, so it might be a good thing to 
> make it common to share.

Yeah, and have the default hook for gimple be similar to the rtl one.


Segher