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]

reload_cse deleting a memory load (zero_extend)


hi guys!

[this is a recap from a conversation I had with rth]

I have the following situation on the PA:

(insn 10 (set (reg:QI 19) -86)
(insn 20 (set (mem:QI X) (reg:QI 19))			;; store into memX
(insn 30 (set (reg:QI 19) (mem:QI X))			;; load from memX
							;; this zero-extends!
(insn 40 (set (reg:DI 19) (lshift:DI (reg:DI 19) 1))	;; use r19 in DI mode

Insn 30 zero-extends into r19 into the full word, so when reload_cse_regs()
deletes insn 30, insn 40 does the wrong thing.

reload_cse_regs() is calling rtx_equal_for_cselib_p() comparing
(reg 19) and (mem X) and they obviously return the same cse hash, because
in QI mode, they contain the same data, however that does not take
into account that a latter insn may use r19 in a wider mode.

Richard had given me this patch to fix the problem:

2001-02-17  Richard Henderson  <rth@redhat.com>

        * reload1.c (reload_cse_simplify_set): Respect LOAD_EXTEND_OP
        when replacing a memory load with a register.

However, it doesn't fix the problem because his magic in
reload_cse_simplify_set() is not run until after the check A, when we
realize the set is ambiguous and delete the insn (see below).

reload_cse_simplify (insn)
     rtx insn;
{
  rtx body = PATTERN (insn);

  if (GET_CODE (body) == SET)
    {
      int count = 0;
      if (reload_cse_noop_set_p (body))		<-- A
        {
          rtx value = SET_DEST (body);
          if (! REG_FUNCTION_VALUE_P (SET_DEST (body)))
            value = 0;
          reload_cse_delete_noop_set (insn, value);	<-- insn gets deleted
          return;
        }

      /* It's not a no-op, but we can try to simplify it.  */
      count += reload_cse_simplify_set (body, insn);	<-- rth's patch

So... I came up with the following patch that merely calls
reload_cse_simplify_set() before reload_cse_noop_set_p() so the insn
gets fixed up before we delete it by mistake.

ok to install?

Aldy

2001-03-09  Aldy Hernandez  <aldyh@redhat.com>

        * reload1.c (reload_cse_simplify): Call reload_cse_simplify_set
        before reload_cse_noop_set_p.

Index: reload1.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reload1.c,v
retrieving revision 1.258
diff -c -p -r1.258 reload1.c
*** reload1.c	2001/03/01 13:35:13	1.258
--- reload1.c	2001/03/09 21:09:54
*************** reload_cse_simplify (insn)
*** 8041,8047 ****
    if (GET_CODE (body) == SET)
      {
        int count = 0;
!       if (reload_cse_noop_set_p (body))
  	{
  	  rtx value = SET_DEST (body);
  	  if (! REG_FUNCTION_VALUE_P (SET_DEST (body)))
--- 8041,8055 ----
    if (GET_CODE (body) == SET)
      {
        int count = 0;
! 
!       /* Simplify even if we may think it is a no-op.
!          We may think a memory load of a value smaller than WORD_SIZE
!          is redundant because we haven't taken into account possible
!          implicit extension.  reload_cse_simplify_set() will bring
!          this out, so it's safer to simplify before we delete.  */
!       count += reload_cse_simplify_set (body, insn);
! 
!       if (!count && reload_cse_noop_set_p (body))
  	{
  	  rtx value = SET_DEST (body);
  	  if (! REG_FUNCTION_VALUE_P (SET_DEST (body)))
*************** reload_cse_simplify (insn)
*** 8049,8057 ****
  	  reload_cse_delete_noop_set (insn, value);
  	  return;
  	}
- 
-       /* It's not a no-op, but we can try to simplify it.  */
-       count += reload_cse_simplify_set (body, insn);
  
        if (count > 0)
  	apply_change_group ();
--- 8057,8062 ----


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