Re: [PATCH][m68k] Partially fix PR target/28181 (movqi from mem to address register)

[Rask Ingemann Lambertsen <rask at sygehus dot dk>]

On Tue, Dec 12, 2006 at 12:27:04PM +0000, Richard Sandiford wrote:
> Hi Rask,
> 
> Rask Ingemann Lambertsen <rask@sygehus.dk> writes:
> > 2006-12-11  Rask Ingemann Lambertsen  <rask@sygehus.dk>
> >
> > 	PR target/28181
> > 	* config/m68k/m68k.h (CANNOT_CHANGE_MODE_CLASS): New.  Forbid
> > 	address registers from changing mode to/from an 8-bit mode.
> 
> I don't think this is the right approach.  I think we should instead
> allow address registers to store bytes, and define secondary_reload_class
> accordingly (plus a few other related changes).

   From m68k_regno_mode_ok(), which implements HARD_REGNO_MODE_OK:

      /* Address Registers, can't hold bytes, can hold aggregate if
         fits in.  */
      if (GET_MODE_SIZE (mode) == 1)
        return false;
      if (regno + GET_MODE_SIZE (mode) / 4 <= 16)
        return true;

   Are you planning to change that also? If not, there will be an
inconsistency. Also, MODES_TIEABLE_P is not defined accordingly.

   I just reread the documentation for HARD_REGNO_MODE_OK and I think
m68k_regno_mode_ok() is more restrictive than it needs to be in that there
is already a movqi pattern to copy QImode values between data registers and
address registers. Limited support for QImode operands on address registers
could probably be added. For example,

(define_insn "*addqi3"
  [(set (match_operand:QI 0 "nonimmediate_operand" "=...,*a,d")
	(plus:QI (match_operand:QI 1 "general_operand" "%...,0,0")
		 (match_operand:QI 2 "general_operand" "...,di,*a")))
  "..."
  "@
  ...
  adda%.w %2,%0
  add%.w %2,%0"
)

   but the condition codes will of course not be usable with the last two
alternatives. Am I overlooking something?
 
> FWIW, I'm gearing up to submit the Coldfire changes from
> csl/sourcerygxx-4_1 branch, and one of those patches does
> what I suggest.  I think it should fix the bug in a way that
> works for 68000 and 68010 too.

   What is the time frame?

-- 
Rask Ingemann Lambertsen