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

login
register
mail settings
Submitter Joern Rennecke
Date Nov. 12, 2010, 4:22 p.m.
Message ID <20101112112249.uy2nt7wlk404w4g4-nzlynne@webmail.spamcop.net>
Download mbox | patch
Permalink /patch/70986/
State New
Headers show

Comments

Joern Rennecke - Nov. 12, 2010, 4:22 p.m.
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.
Joern Rennecke - Nov. 19, 2010, 3:47 a.m.
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.
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.
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.

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);