diff mbox

RFA: Fix crx --enable-werror-always build

Message ID 20101112112249.uy2nt7wlk404w4g4-nzlynne@webmail.spamcop.net
State New
Headers show

Commit Message

Joern Rennecke Nov. 12, 2010, 4:22 p.m. UTC 
Built on i686-pc-linux-gnu with gcc (GCC) 4.6.0 20101111 (experimental) .
2010-11-12  Joern Rennecke  <amylaar@spamcop.net>

	PR target/46434
	* config/crx/crx.c (crx_addr_reg_p): Use reg.
	(crx_expand_epilogue): Remove unused variable.

Comments

Joern Rennecke Nov. 19, 2010, 3:47 a.m. UTC | #1 
Ping?
This patch hasn't been reviewed for a week:
http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01303.html
Richard Henderson Nov. 19, 2010, 6:34 p.m. UTC | #2 
On 11/12/2010 08:22 AM, Joern Rennecke wrote:
> +  gcc_assert (REGNO (reg) != CC_REGNUM);

This is the only change I'm unsure about.  I wonder if instead
the previous IF was supposed to check REG instead of ADDR_REG.

Alternately, 

  if (REGNO (reg) >= LO_REGNUM && REGNO (reg) <= CC_REGNUM)
    return FALSE;

The expand_prologue change is ok, obviously.


r~
Joern Rennecke Nov. 19, 2010, 7:21 p.m. UTC | #3 
Quoting Richard Henderson <rth@redhat.com>:

> On 11/12/2010 08:22 AM, Joern Rennecke wrote:
>> +  gcc_assert (REGNO (reg) != CC_REGNUM);
>
> This is the only change I'm unsure about.  I wonder if instead
> the previous IF was supposed to check REG instead of ADDR_REG.

That seems unlikely, because then the

            && GET_MODE_SIZE (GET_MODE (SUBREG_REG (addr_reg)))
            <= UNITS_PER_WORD))

check would be redundant.
>
> Alternately,
>
>   if (REGNO (reg) >= LO_REGNUM && REGNO (reg) <= CC_REGNUM)
>     return FALSE;

The hi / lo registers could plausibly be reloaded - the
mov<mode>_regs pattern has an alternative for that.  If it's a
good idea to allow such addresses before reload should mostly
be a code quality issue, so I would suppose the conservative
thing is to continue to allow it.

But I would agree, if REGNO (reg) can actually be CC_REGNUM in this
place, the return value should be FALSE.
diff mbox

Patch

Index: config/crx/crx.c
===================================================================
--- config/crx/crx.c	(revision 166609)
+++ config/crx/crx.c	(working copy)
@@ -613,6 +613,8 @@  static int crx_addr_reg_p (rtx addr_reg)
       return FALSE;
     }
 
+  gcc_assert (REGNO (reg) != CC_REGNUM);
+
   return TRUE;
 }
 
@@ -1439,17 +1441,12 @@  crx_expand_prologue (void)
 void
 crx_expand_epilogue (void)
 {
-  rtx return_reg;
-
   /* Nonzero if we need to return and pop only RA. This will generate a
    * different insn. This differentiate is for the peepholes for call as last
    * statement in function. */
   int only_popret_RA = (save_regs[RETURN_ADDRESS_REGNUM]
 			&& (sum_regs == UNITS_PER_WORD));
 
-  /* Return register.  */
-  return_reg = gen_rtx_REG (Pmode, RETURN_ADDRESS_REGNUM);
-
   if (frame_pointer_needed)
     /* Restore the stack pointer with the frame pointers value */
     emit_move_insn (stack_pointer_rtx, frame_pointer_rtx);