DFA scheduler buglet

[law at redhat dot com]

I should have sent this a while ago, but I've been on the road a lot lately...

For older PA processors the DFA based pipeline description does not provide
significant opportunities to improve code.  However, it's still profitable 
to convert those old descriptions to the DFA model to get software pipelining
in the future.

To help avoid introducing performance regressions when building DFA
descriptions for these older processors the DFA descriptions are designed
to mimic the old descriptions with 100% accuracy.  This is tested by 
running large bodies of code through the compiler with and without the
DFA descriptions and comparing the output.

This process is interesting in that it will highlight buglets in the
DFA processor description and/or differences in how the generic DFA
scheduler works when compared to the non-DFA scheduler.

One of those differences is (IMHO) clearly a bug in the generic DFA
code.  In schedule_block we have code like this:


              if (ready.n_ready == 0 || !can_issue_more
                  || state_dead_lock_p (curr_state)
                  || !(*current_sched_info->schedule_more_p) ())
                break;

              /* Select and remove the insn from the ready list.  */
              insn = choose_ready (&ready);

              if (recog_memoized (insn) < 0)
                {
                  if (!first_cycle_insn_p
                      && (GET_CODE (PATTERN (insn)) == ASM_INPUT
                          || asm_noperands (PATTERN (insn)) >= 0))
                    /* This is asm insn which is tryed to be issued on the
                       cycle not first.  Issue it on the next cycle.  */
                    cost = 1;
                  else
                    /* A USE insn, or something else we don't need to
                       understand.  We can't pass these directly to
                       state_transition because it will trigger a
                       fatal error for unrecognizable insns.  */
                    cost = 0;
                }
              else
                {
                  cost = state_transition (curr_state, insn);

		  [ ... ]


          if (cost >= 1)
            {
              queue_insn (insn, cost);
              continue;
            }

          if (! (*current_sched_info->can_schedule_ready_p) (insn))
            goto next;



Note carefully that we're passing "curr_state" into state_transition, but 
that we may not actually issue the ready insn.  This results in insns
which are not issued causing unexpected state transitions and ultimately
resulting in odd schedules.

It seems to me that this code should compute the transition cost using a
temporary copy of the current state (thus state updates happen in the temp
state variable).  Then when we commit to issuing the instruction we can
commit any state transitions.

Comments?

jeff