[PATCH] sched: Do not move expensive insns speculatively (PR68664)

[Segher Boessenkool]

Hi Jeff,

On Fri, Feb 03, 2017 at 04:31:58PM -0700, Jeff Law wrote:
> >+@deftypefn {Target Hook} bool TARGET_SCHED_CAN_SPECULATE_INSN (rtx_insn 
> >*@var{insn})
> >+Some instructions should never be speculated by the schedulers, usually
> >+ because the instruction is too expensive to get this wrong.  This hook
> >+ should return @code{false} if @var{insn} should not be speculated.
> >+@end deftypefn
> Consider adding something like this:
> 
> Define this hook to return false for instructions which are not fully 
> modeled by the pipeline description to avoid DFA size explosion. 
> Otherwise the scheduler may erroneously speculate those instructions 
> into a pipeline bubble that is too small which may severely impact 
> performance.

Well, it speculates it even _if_ you correctly model it, currently
anyway.  But I'll write something similar, good idea.

> >+  if (INSN_BB (insn) != target_bb && IS_SPECULATIVE_INSN (insn))
> >+    {
> >+      /* Cannot schedule this insn unless all operands are live.  */
> >+      if (!check_live (insn, INSN_BB (insn)))
> >+	return 0;
> >+
> >+      /* Should not move expensive instructions speculatively.  */
> >+      if (GET_CODE (PATTERN (insn)) != CLOBBER
> >+	  && !targetm.sched.can_speculate_insn (insn))
> >+	return 0;
> >+    }
> You probably want to filter both CLOBBER and USE insns.

I had that originally (and more checks), but only CLOBBER can be
speculated and there is that IS_SPECULATIVE_INSN check right before.
I had the CLOBBER check in the rs6000 hook first -- get_attr_type
will die on it -- but I figured it makes more sense in the generic
code.

> OK with those two nits addressed.

Thanks!


Segher