Hi,

We have this charming move_insn function in haifa-sched.c:

/* Move INSN.  Reemit notes if needed.

   Return the last insn emitted by the scheduler, which is the
   return value from the first call to reemit_notes.  */

static rtx
move_insn (rtx insn, rtx last)
{
  rtx retval = NULL;

  move_insn1 (insn, last);

  /* If this is the first call to reemit_notes, then record
     its return value.  */
  if (retval == NULL_RTX)
    retval = reemit_notes (insn, insn);
  else
    reemit_notes (insn, insn);

  SCHED_GROUP_P (insn) = 0;

  return retval;
}


First of all, move_insn1 should really be replaced with reorder_insns,
but OK, whatever, this code is older.

But what is this weird thing move_insn is doing with retval?  It is
pretty obvious that retval is NULL_RTX in "if (retval == NULL_RTX)",
so I wonder what the purpose was of this code.  Archeology hasn't
helped me much.  retval was introduced in revision 1.8 by Jeff:

1.1           law       6501: 
                        6502: static rtx
                        6503: move_insn (insn, last)
                        6504:      rtx insn, last;
                        6505: {
1.8          |law       6506:   rtx retval = NULL;
1.1           law       6507: 
1.8          |law       6508:   /* If INSN has SCHED_GROUP_P set, then issue it 
and any other
             |          6509:      insns with SCHED_GROUP_P set first.  */
1.1           law       6510:   while (SCHED_GROUP_P (insn))
                        6511:     {
                        6512:       rtx prev = PREV_INSN (insn);
1.8          |law       6513: 
             |          6514:       /* Move a SCHED_GROUP_P insn.  */
1.1           law       6515:       move_insn1 (insn, last);
1.8          |law       6516:       /* If this is the first call to 
reemit_notes, then record
             |          6517:         its return value.  */
             |          6518:       if (retval == NULL_RTX)
             |          6519:        retval = reemit_notes (insn, insn);
             |          6520:       else
             |          6521:        reemit_notes (insn, insn);
1.1           law       6522:       insn = prev;
                        6523:     }
                        6524: 
1.8          |law       6525:   /* Now move the first non SCHED_GROUP_P insn.  
*/
1.1           law       6526:   move_insn1 (insn, last);
1.8          |law       6527: 
             |          6528:   /* If this is the first call to reemit_notes, 
then record
             |          6529:      its return value.  */
             |          6530:   if (retval == NULL_RTX)
             |          6531:     retval = reemit_notes (insn, insn);
             |          6532:   else
             |          6533:     reemit_notes (insn, insn);
             |          6534: 
             |          6535:   return retval;
1.1           law       6536: }


But in revision 1.215, the itanium-sched-branch was merged and the
function was simplified to:

        law       1765: 
                        1766: static rtx
                        1767: move_insn (insn, last)
                        1768:      rtx insn, last;
                        1769: {
1.8           law       1770:   rtx retval = NULL;
1.1           law       1771: 
1.8           law       1772:   /* Now move the first non SCHED_GROUP_P insn.  
*/
1.1           law       1773:   move_insn1 (insn, last);
1.8           law       1774: 
                        1775:   /* If this is the first call to reemit_notes, 
then record
                        1776:      its return value.  */
                        1777:   if (retval == NULL_RTX)
                        1778:     retval = reemit_notes (insn, insn);
                        1779:   else
                        1780:     reemit_notes (insn, insn);
                        1781: 
1.215        |vmakarov  1782:   SCHED_GROUP_P (insn) = 0;
             |          1783: 
1.8           law       1784:   return retval;
1.1           law       1785: }


The only ChangeLog entry I can find for this change is:

        * sched-rgn.c (init_ready_list, can_schedule_ready_p,
        add_branch_dependences): Ignore schedule groups.

        * sched-ebb.c (init_ready_list): Ditto.

        * (move_insn, set_priorities): Ditto.

Vlad, can you explain this change?  Can I safely clean up this function,
or should we still avoid calling reemit_notes more than once for each
insn???

Gr.
Steven

Reply via email to