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