-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

This has gone latent on the trunk, but the underlying issue hasn't been
resolved.

ira.c::update_equiv_regs can create REG_EQUIV notes for equivalences
which are local to a block rather than the traditional function-wide
equivalences we typically work with.

This occurs when we have an insn that loads a pseudo from a MEM and the
pseudo is used within only a single block and the MEM remains unchanged
through the life of the pseudo.

Starting with the assumption that we're going to create a block local
pseudo under the rules noted above, consider this RTL:

(set (reg X) (some address))
(set (reg Y) (mem (reg X)))
(use Y)


We're going to create an equivalence between (reg Y) and its memory
location in update_equiv_regs.  Assume IRA is able to allocate a hard
reg for reg X, but not reg Y.

reload's strategy in this situation will be to remove the insn which
creates the equivalence between reg Y and the memory location.  Uses of
reg Y will be replaced with the equivalent memory location.

That's all fine and good, except reload uses delete_dead_insn, which
deletes the equivalencing insn, but also recursively tries to remove the
prior insn if it becomes dead as a result of removing the equivalencing
insn.

Anyway, continuing with our example, reg X gets a hard reg, so our RTL
will look something like

(set (reg 0) (some address))
(set (reg Y) (mem (reg 0)))
(use Y)

Then we remove the equivalencing insn resulting in

(set (reg 0) (some address)
(use Y)

And we recurse from delete_dead_insn and determine that the first insn
was dead as well, so it gets removed leaving:

(use Y)

We then replace Y with its equivalent memory location

(use (mem (reg 0))

At which point we lose because hard reg 0 is no longer initialized.


The code in question is literally 20 years old and predates running any
real dead code elimination after reload.  ISTM the right thing to do is
stop using delete_dead_insn in this code and let the post-reload DCE
pass do its job.  That allows us to continue to record the block local
equivalence.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu.
Testcase is included, even though the bug has gone latent on the trunk.

OK?


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJN28/5AAoJEBRtltQi2kC79hAH/13usH0o6sNusDXHfuNgfFfu
+qpQpXo9P1UdyOoSU8pycb4IkR8Wi675xpwQw0pgMG7EVZ6AbE+FvhEO5Czu8OJz
1QBfNxCkWgNCDjt7SG4YqNBsk1HBs2BjTxFv66DkhquslxYZrftWSgoGcywAU6Qn
5eaRaOfk2hLQ52fm+X3MrWrLUjIf+Tg2HFegwCNHNRvTtpeKFm5RBjJcdpn1PWcF
1Hb8vjlkMK8ttYF1uySKm+q8q5wxlq6hBAWfiBtMZz2KFL6+Vr+Y9LfvH/4eE9e9
ZGx/2bqTsxFYCE6PSPA+7rIcNKzFHLSAAA0CfVy9fk7p9vnXJL8VE+8F0A5Pj8o=
=fYFt
-----END PGP SIGNATURE-----
        PR middle-end/48770
        * ira.c (update_equiv_regs): Update comment.
        * reload1.c (reload): Don't call delete_dead_insn to delete
        insns which create equivalences.

        PR middle-end/48770
        * gcc.dg/pr48770.c: New test.

Index: testsuite/gcc.dg/pr48770.c
===================================================================
*** testsuite/gcc.dg/pr48770.c  (revision 0)
--- testsuite/gcc.dg/pr48770.c  (revision 0)
***************
*** 0 ****
--- 1,21 ----
+ /* { dg-do run } */
+ /* { dg-options "-O -fprofile-arcs -fPIC -fno-dce -fno-forward-propagate" } */
+ 
+ int test_goto2 (int f)
+ {
+   int i;
+   for (i = 0; ({_Bool a = i < 10;a;}); i++)
+   {
+     if (i == f)
+       goto lab2;
+   }
+   return 4;
+ lab2:
+   return 8;
+ }
+ 
+ int main ()
+ {
+   test_goto2 (30);
+   return 0;
+ }
Index: ira.c
===================================================================
*** ira.c       (revision 174066)
--- ira.c       (working copy)
*************** update_equiv_regs (void)
*** 2842,2848 ****
             If we don't have a REG_EQUIV note, see if this insn is loading
             a register used only in one basic block from a MEM.  If so, and the
             MEM remains unchanged for the life of the register, add a REG_EQUIV
!            note.  */
  
          note = find_reg_note (insn, REG_EQUIV, NULL_RTX);
  
--- 2842,2848 ----
             If we don't have a REG_EQUIV note, see if this insn is loading
             a register used only in one basic block from a MEM.  If so, and the
             MEM remains unchanged for the life of the register, add a REG_EQUIV
!            note, this creates a block local equivalence.  */
  
          note = find_reg_note (insn, REG_EQUIV, NULL_RTX);
  
Index: reload1.c
===================================================================
*** reload1.c   (revision 174086)
--- reload1.c   (working copy)
*************** reload (rtx first, int global)
*** 1011,1021 ****
        mark_elimination (ep->from, ep->to);
  
    /* If a pseudo has no hard reg, delete the insns that made the equivalence.
!      If that insn didn't set the register (i.e., it copied the register to
!      memory), just delete that insn instead of the equivalencing insn plus
!      anything now dead.  If we call delete_dead_insn on that insn, we may
!      delete the insn that actually sets the register if the register dies
!      there and that is incorrect.  */
  
    for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
      {
--- 1011,1025 ----
        mark_elimination (ep->from, ep->to);
  
    /* If a pseudo has no hard reg, delete the insns that made the equivalence.
!      We used to use delete_dead_insn which would recursively delete anything
!      else now dead.    That is incorrect if that insn didn't set the register
!      (i.e., it copied the register to memory); it is also incorrect for a
!      block local equivalence as the memory address may refer to another
!      pseudo which still needs proper initialization. 
! 
!      That code predates running dead code elimination after reload, so rather
!      that trying to identify all the dead code here (and get it wrong), just
!      delete the equivalencing insn and let DCE do its job.  */
  
    for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
      {
*************** reload (rtx first, int global)
*** 1034,1041 ****
              if (NOTE_P (equiv_insn)
                  || can_throw_internal (equiv_insn))
                ;
-             else if (reg_set_p (regno_reg_rtx[i], PATTERN (equiv_insn)))
-               delete_dead_insn (equiv_insn);
              else
                SET_INSN_DELETED (equiv_insn);
            }
--- 1038,1043 ----

Reply via email to