This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[3.4 PATCH] PR opt/17581: Pseudo escaping from libcall block


The following patch resolves PR rtl-optimization/17581 a wrong-code
regression affecting that affects the gcc-3_4-branch.  The problem
is present but latent for the PR's testcase on mainline.

The problem is that pseudo registers assigned values within a libcall
block, with the exception of the final REG_RETVAL assignment are assumed
local to the libcall block.  These "local" pesudos should not be
propagated by CSE or GCSE out of the libcall block, as whenever the
result of a libcall block is discovered to be dead, all of the
instructions of that block are deleted.  For this PR, cselib breaks
the rules, and propagates a libcall pseudo into a following insn,
and when the original libcall gets deleted, we end up using a pseudo
which is never assigned a value.

The problem comes from the definition of the bounds of a libcall
block.  As described above, all pseudos assigned a value except
for the last insn (bearing the REG_RETVAL) note are considered local.
However, for the RHS of "set" instruction, this last insn must be
considered part of the libcall block.


Consider the following libcall block (from the PR):

(insn 24 21 22 pr17581-1.c:12 (clobber (reg:DI 62 [ tmp ])) -1 (nil)
    (insn_list:REG_LIBCALL 25 (nil)))

(insn 22 24 23 pr17581-1.c:12 (parallel [
            (set (subreg:SI (reg:DI 62 [ tmp ]) 0)
                (ior:SI (subreg:SI (reg/v:DI 60 [ tmp ]) 0)
                    (const_int 2 [0x2])))
            (clobber (reg:CC 17 flags))
        ]) 209 {*iorsi_1} (nil)
    (expr_list:REG_NO_CONFLICT (reg/v:DI 60 [ tmp ])
        (nil)))

(insn 23 22 25 pr17581-1.c:12 (set (subreg:SI (reg:DI 62 [ tmp ]) 4)
        (subreg:SI (reg/v:DI 60 [ tmp ]) 4)) 36 {*movsi_1} (nil)
    (expr_list:REG_NO_CONFLICT (reg/v:DI 60 [ tmp ])
        (nil)))

(insn 25 23 27 pr17581-1.c:12 (set (reg/v:DI 60 [ tmp ])
        (reg:DI 62 [ tmp ])) 58 {*movdi_2} (nil)
    (insn_list:REG_RETVAL 24 (expr_list:REG_EQUAL (ior:DI (reg/v:DI 60 [ tmp ])
                (const_int 2 [0x2]))
            (nil))))

In this block, notice that reg:DI 62 is local to the libcall block,
but reg:DI 60 is a regular global pseudo.  This confusion of definitions
led to cselib_process_insn thinking that "insn 25" is not part of the
libcall block, and therefore recording that "reg 60" is equivalent to
"reg 62" in the CSE hashtables.  Uses of reg60 later in the function
therefore get replaced by reg 62, which in turn causes the problem
when the above instructions get deleted.

The fix is that from cselib_process_insn's prespective the instruction
with the REG_RETVAL note should be part of the libcall block, and the
cselib_current_insn_in_libcall flag shouldn't be cleared until after
cselib_record_sets has been called.


To counteract any minor performance loss by not recording the REG_RETVAL
instructions in the cselib hash tables, the patch below includes a
refinement to GCSE's do_local_cprop that allows us to propagate
constants out of SET_SRCs in a libcall block.  It's only expressions that
contain pseudos that we have to be careful about, so if the final insn
is "(set (reg:DI 60) (const_int 0))" it's safe to use the const0_rtx
equivalence.

Finally, this patch also changes a call to replace_rtx to the more
appropriate simplify_replace_rtx.  Whilst investigating this PR, I
was originally suspicious of the invalid RTL that was attached to a
REG_EQUAL note, (ior:DI (const_int 0) (const_int 1)).  This change
prevents us creating such a dubious RTX, but doesn't cure the PR.


The following patch has been tested against the gcc-3_4-branch on
i686-pc-linux-gnu with a full "make bootstrap", all default languages,
and regression tested with a top-level "make -k check" with no new
failures.  The attached testcase fails without this patch but passes
with it.  Bootstrap and regression testing is in underway for mainline,
but I thought I'd post what I had as we're approach the 3.4.3 freeze.

Ok for mainline and the gcc-3_4-branch?



2004-10-28  Roger Sayle  <roger@eyesopen.com>

	PR rtl-optimization/17581
	* cselib.c (cselib_process_insn): The last instruction of a libcall
	block, with the REG_RETVAL note, should be considered in the libcall.
	* gcse.c (do_local_cprop): Allow constants to be propagated outside
	of libcall blocks.
	(adjust_libcall_notes): Use simplify_replace_rtx instead of
	replace_rtx to avoid creating invalid RTL in REG_RETVAL notes.

	* gcc.dg/pr17581-1.c: New test case.


Index: cselib.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cselib.c,v
retrieving revision 1.32.4.6
diff -c -3 -p -r1.32.4.6 cselib.c
*** cselib.c	13 Sep 2004 08:52:43 -0000	1.32.4.6
--- cselib.c	28 Oct 2004 18:41:27 -0000
*************** cselib_process_insn (rtx insn)
*** 1339,1346 ****

    if (find_reg_note (insn, REG_LIBCALL, NULL))
      cselib_current_insn_in_libcall = true;
-   if (find_reg_note (insn, REG_RETVAL, NULL))
-     cselib_current_insn_in_libcall = false;
    cselib_current_insn = insn;

    /* Forget everything at a CODE_LABEL, a volatile asm, or a setjmp.  */
--- 1339,1344 ----
*************** cselib_process_insn (rtx insn)
*** 1351,1362 ****
--- 1349,1364 ----
  	  && GET_CODE (PATTERN (insn)) == ASM_OPERANDS
  	  && MEM_VOLATILE_P (PATTERN (insn))))
      {
+       if (find_reg_note (insn, REG_RETVAL, NULL))
+         cselib_current_insn_in_libcall = false;
        clear_table ();
        return;
      }

    if (! INSN_P (insn))
      {
+       if (find_reg_note (insn, REG_RETVAL, NULL))
+         cselib_current_insn_in_libcall = false;
        cselib_current_insn = 0;
        return;
      }
*************** cselib_process_insn (rtx insn)
*** 1392,1397 ****
--- 1394,1401 ----
        if (GET_CODE (XEXP (x, 0)) == CLOBBER)
  	cselib_invalidate_rtx (XEXP (XEXP (x, 0), 0));

+   if (find_reg_note (insn, REG_RETVAL, NULL))
+     cselib_current_insn_in_libcall = false;
    cselib_current_insn = 0;

    if (n_useless_values > MAX_USELESS_VALUES)
Index: gcse.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gcse.c,v
retrieving revision 1.288.2.8
diff -c -3 -p -r1.288.2.8 gcse.c
*** gcse.c	1 Sep 2004 20:30:28 -0000	1.288.2.8
--- gcse.c	28 Oct 2004 18:41:30 -0000
*************** do_local_cprop (rtx x, rtx insn, int alt
*** 4303,4309 ****
  	  rtx this_rtx = l->loc;
  	  rtx note;

! 	  if (l->in_libcall)
  	    continue;

  	  if (gcse_constant_p (this_rtx))
--- 4303,4310 ----
  	  rtx this_rtx = l->loc;
  	  rtx note;

! 	  /* Don't CSE non-constant values out of libcall blocks.  */
! 	  if (l->in_libcall && ! CONSTANT_P (this_rtx))
  	    continue;

  	  if (gcse_constant_p (this_rtx))
*************** adjust_libcall_notes (rtx oldreg, rtx ne
*** 4388,4394 ****
  	      return true;
  	    }
  	}
!       XEXP (note, 0) = replace_rtx (XEXP (note, 0), oldreg, newval);
        insn = end;
      }
    return true;
--- 4389,4395 ----
  	      return true;
  	    }
  	}
!       XEXP (note, 0) = simplify_replace_rtx (XEXP (note, 0), oldreg, newval);
        insn = end;
      }
    return true;


/* PR rtl-optimization/17581 */
/* { dg-do run } */
/* { dg-options "-O2" } */

int foo(int x)
{
  unsigned long long tmp = 0;

  switch(x) {
    case 21:
      tmp |= 1;
      tmp |= 2;
      tmp |= 8;
      break;
    default:
      break;
  }

  return (int)tmp;
}

int main()
{
  if (foo(21) != 11)
    abort ();
  return 0;
}


Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]