More on the reload problem

Jeffrey A Law law@cygnus.com
Mon Nov 2 04:08:00 GMT 1998


Remember this issue:
  I think we need to fix forget_old_reloads_1 to handle cases where we
  store into a pseudo which happens to be allocated to a hard reg that
  has also been used as a spill reg.  Imagine

          Use reg 0 as an output reload for some value V
 
          Psuedo X is made live and is allocated to reg 0
          [ ... ]
          Pseudo X dies

          Use of V which needs an input reload.

  In this case we might try and use reg0 to satisfy the input reload
  even though it no longer holds the value we need.

Your reply:

  I'm not convinced yet that anything is broken.  The situation you describe
  could already occur with the old reload code, since it has some simple code
  to avoid spilling within one basic block.
  In the above example, if pseudo X is allocated to reg 0, its rtl will have
  been modified to look like hard reg 0 by the time forget_old_reloads_1 runs.
  And in that case, the following piece of code in find_old_reloads_1 should
  run.

      nr = HARD_REGNO_NREGS (regno, GET_MODE (x));
      /* Storing into a spilled-reg invalidates its contents.
         This can happen if a block-local pseudo is allocated to that reg
         and it wasn't spilled because this block's total need is 0.
         Then some insn might have an optional reload and use this reg.  */
      for (i = 0; i < nr; i++)
        /* But don't do this if the reg actually serves as an output
           reload reg in the current instruction.  */
        if (n_reloads == 0
            || ! TEST_HARD_REG_BIT (reg_is_output_reload, regno + i))
          CLEAR_HARD_REG_BIT (reg_reloaded_valid, regno + i);

  It's quite possible that I've missed something, but so far I don't see where
  things could break.


I think I know where it breaks :-)

Going back to last night's example.  We've got the following insns as we
exit from emit_reload_insns for insn 170.

(insn 531 167 512 (set (reg:SI 5 %edi)
        (reg:SI 108)) -1 (nil)
    (nil))

(insn 512 531 537 (set (reg:SI 1 %edx)
        (reg:SI 5 %edi)) 54 {movsi+2} (nil)
    (nil))

(insn 537 512 170 (set (reg:SI 0 %eax)
        (reg:SI 60)) -1 (nil)
    (nil))

(insn 170 537 534 (parallel[ 
            (set (reg/v:SI 29)
                (unspec:SI[ 
                        (mem:BLK (reg/v:SI 29) 0)
                        (const_int 4)
                        (reg:SI 60)
                    ]  0))
            (clobber (reg:SI 60))
        ] ) 390 {strlensi_unroll5} (nil)
    (nil))

(insn 534 170 172 (set (reg/v:SI 29)
        (reg:SI 1 %edx)) -1 (nil)
    (nil))

(insn 172 534 177 (set (reg/v:SI 29)
        (minus:SI (reg/v:SI 29)
            (reg:SI 108))) 157 {subsi3+1} (insn_list 170 (nil))


First, note that spill_reg_store[%edi] points to insn 512 and
spill_reg_stored_to[%edi] is (reg:SI 29).  This makes sense if you happen
to know that insn 512 originally stored into reg29 :-)  Refer to last night's
message.

So, one would think that when we call forget_old_reloads for insn 170 and
its associated reload insns (537, 534) that we would invalidate
spill_reg_stored_to[%edi] since the value in (reg 29) is changing.

We call forget_old_reloads_1 three times for insn 170.  Once with the argument
(reg 29), then with (reg %edx), and finally with (reg %eax).

*NONE* of these calls will invalidate the info in spill_reg_store[%edi]
(either directly or indirectly).  Why? Let's look at forget_old_reloads_1:

static void
forget_old_reloads_1 (x, ignored)
     rtx x;
     rtx ignored ATTRIBUTE_UNUSED;
{
  register int regno;
  int nr;
  int offset = 0;

  /* note_stores does give us subregs of hard regs.  */
  while (GET_CODE (x) == SUBREG)
    {
      offset += SUBREG_WORD (x);
      x = SUBREG_REG (x);
    }

  if (GET_CODE (x) != REG)
    return;

  regno = REGNO (x) + offset;

  if (regno >= FIRST_PSEUDO_REGISTER)
    nr = 1;
  else
    {
      int i;
      nr = HARD_REGNO_NREGS (regno, GET_MODE (x));
      /* Storing into a spilled-reg invalidates its contents.
         This can happen if a block-local pseudo is allocated to that reg
         and it wasn't spilled because this block's total need is 0.
         Then some insn might have an optional reload and use this reg.  */
      for (i = 0; i < nr; i++)
        /* But don't do this if the reg actually serves as an output
           reload reg in the current instruction.  */
        if (n_reloads == 0
            || ! TEST_HARD_REG_BIT (reg_is_output_reload, regno + i))
          CLEAR_HARD_REG_BIT (reg_reloaded_valid, regno + i);
    }

  /* Since value of X has changed,
     forget any value previously copied from it.  */

  while (nr-- > 0)
    /* But don't forget a copy if this is the output reload
       that establishes the copy's validity.  */
    if (n_reloads == 0 || reg_has_output_reload[regno + nr] == 0)
      reg_last_reload_reg[regno + nr] = 0;
}


When called with (reg 29) we do nothing because we have an output reload
for (reg 29).  Nada, zilch.  Danger danger.

The call for (reg %eax) isn't interesting since it's dealing with an unrelated
value.

Then we call with (reg %edx).  There's no code to map back the the fact that
we need to kill spill_reg_store[%edi] -- we clear reg_last_reload_reg[%edx},
but that has no effect on the info for %edi.


One obvious solution is to walk over spill_reg_stored_to when X is a pseudo
and see if any entries match the pseudo.  If they do, then clear them
(and their associated spill_reg_store entry).

I'm off to Flordia.  If I'm lucky, I may be online briefly either tomorrow
or tuesday.

jeff



  



More information about the Gcc-bugs mailing list