Message ID | 4C36F1BC.8080401@codesourcery.com |
---|---|
State | New |
Headers | show |
Fix subreg handling of HARD_FRAME_POINTER_REGNUM and revert a Thumb patch which seems to work around the problem: http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00756.html Bernd
Fix subreg handling of HARD_FRAME_POINTER_REGNUM (not an ARM patch, needs review) and revert a Thumb patch which seems to work around the problem: http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00756.html Bernd
On 07/23/2010 03:21 AM, Bernd Schmidt wrote: > Fix subreg handling of HARD_FRAME_POINTER_REGNUM (not an ARM patch, > needs review) and revert a Thumb patch which seems to work around the > problem: > > http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00756.html The reload patch looks ok. The arm patch looks... odd. What use is a define_expand that only aborts? Why not delete the pattern entirely, particularly since it isn't a pattern name known by the middle-end? r~
On 07/28/2010 06:57 PM, Richard Henderson wrote: > On 07/23/2010 03:21 AM, Bernd Schmidt wrote: >> Fix subreg handling of HARD_FRAME_POINTER_REGNUM (not an ARM patch, >> needs review) and revert a Thumb patch which seems to work around the >> problem: >> >> http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00756.html > > The reload patch looks ok. Thanks! > The arm patch looks... odd. What use is a define_expand that > only aborts? Why not delete the pattern entirely, particularly > since it isn't a pattern name known by the middle-end? Yeah. I'm just reverting it to a previous state; I was hoping the other Richard would either point out why we need the pattern or suggest we remove it. Bernd
On Wed, 2010-07-28 at 19:03 +0200, Bernd Schmidt wrote: > On 07/28/2010 06:57 PM, Richard Henderson wrote: > > On 07/23/2010 03:21 AM, Bernd Schmidt wrote: > >> Fix subreg handling of HARD_FRAME_POINTER_REGNUM (not an ARM patch, > >> needs review) and revert a Thumb patch which seems to work around the > >> problem: > >> > >> http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00756.html > > > > The reload patch looks ok. > > Thanks! > > > The arm patch looks... odd. What use is a define_expand that > > only aborts? Why not delete the pattern entirely, particularly > > since it isn't a pattern name known by the middle-end? > > Yeah. I'm just reverting it to a previous state; I was hoping the other > Richard would either point out why we need the pattern or suggest we > remove it. This is part of the reload_{in,out}hi sequences. These have always blown my mind away and I've never fully understood when and why these are really needed. In thumb1 you potentially need some sort of recovery sequence if the compiler needs to spill a register held in r8-r14 (since these registers can't be stored directly to memory), but why it really needs a secondary reload escapes me -- the compiler should just copy the register to a lo-reg and then store that (or vice-versa for a load). I suggest we get rid of the pattern entirely, and the code chain that can end up calling it (ie thumb_reload_out_hi. We should probably also take a look at the reload code in arm.md at some point: it still uses the now deprecated reload support hooks and the infrastructure that (I think) Joern wrote a few years back has never been implemented on ARM. R.
Index: rtlanal.c =================================================================== --- rtlanal.c (revision 161987) +++ rtlanal.c (working copy) @@ -3297,8 +3297,7 @@ simplify_subreg_regno (unsigned int xreg /* We shouldn't simplify stack-related registers. */ if ((!reload_completed || frame_pointer_needed) - && (xregno == FRAME_POINTER_REGNUM - || xregno == HARD_FRAME_POINTER_REGNUM)) + && xregno == FRAME_POINTER_REGNUM) return -1; if (FRAME_POINTER_REGNUM != ARG_POINTER_REGNUM Index: config/arm/arm.md =================================================================== --- config/arm/arm.md (revision 161987) +++ config/arm/arm.md (working copy) @@ -5666,17 +5666,9 @@ (define_expand "thumb_movhi_clobber" (match_operand:HI 1 "register_operand" "")) (clobber (match_operand:DI 2 "register_operand" ""))] "TARGET_THUMB1" - " - if (strict_memory_address_p (HImode, XEXP (operands[0], 0)) - && REGNO (operands[1]) <= LAST_LO_REGNUM) - { - emit_insn (gen_movhi (operands[0], operands[1])); - DONE; - } - /* XXX Fixme, need to handle other cases here as well. */ +{ gcc_unreachable (); - " -) +}) ;; We use a DImode scratch because we may occasionally need an additional ;; temporary if the address isn't offsettable -- push_reload doesn't seem