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]
Other format: [Raw text]

Re: [m68k 09/13] Convert some text peepholes


Roman Zippel <zippel@linux-m68k.org> writes:
> On Tue, 30 Jan 2007, Richard Sandiford wrote:
>> I have no strong feelings here (it's !TARGET_COLDFIRE ;)) but I think it
>> would be better stylistically to have a new constraint that behaves like
>> 'K' before and during reload, and like 'n' after reload.  That way,
>> if some post-reload pass looks at the constraints of a reloaded second
>> insn, it will know that moveq constants are now allowed.  It's a minor
>> point though.
>
> BTW I think something similiar is needed for ColdFire as well, if we want 
> to avoid the unspec (which I'd really like to).

I think it's 680x0 only.  The ColdFire moves patterns don't play the
same moveq trick as the 680x0 code.  (Don't ask me why.)

>> One thing the patch I posted did was use adjust_address* family
>> of functions to create the new MEM.  Hard-coding the MEM in the
>> output pattern will lose the alias information on the original MEM
>> (including that set up by get_frame_mem).  The same comment applies
>> to the other peepholes.
>
> Below is new version of the patch which does this, I also added the CF 
> variant for a single byte push. (Ligthly tested.)

It still contains hard-coded MEMs.  E.g.:

> -(define_peephole
> +(define_peephole2
>    [(set (reg:SI SP_REG) (plus:SI (reg:SI SP_REG) (const_int 4)))
> -   (set (match_operand:DF 0 "register_operand" "=f")
> -	(match_operand:DF 1 "register_operand" "ad"))]
> -  "FP_REG_P (operands[0]) && ! FP_REG_P (operands[1])"
> -{
> -  rtx xoperands[2];
> -  xoperands[1] = gen_rtx_REG (SImode, REGNO (operands[1]) + 1);
> -  output_asm_insn ("move%.l %1,%@", xoperands);
> -  output_asm_insn ("move%.l %1,%-", operands);
> -  return "fmove%.d %+,%0";
> -})
> +   (set (match_operand:DF 0 "register_operand" "")
> +	(match_operand:DF 1 "register_operand" ""))]
> +  "FP_REG_P (operands[0]) && !FP_REG_P (operands[1])"
> +  [(set (mem:SI (reg:SI SP_REG)) (match_dup 1))
> +   (set (mem:SI (pre_dec:SI (reg:SI SP_REG))) (match_dup 2))
> +   (set (match_dup 0) (mem:DF (post_inc:SI (reg:SI SP_REG))))]
> +  "split_di(operands + 1, 1, operands + 1, operands + 2);")

...here.

It's your call, but seeing as the patch I posted has been approved,
and is only waiting on an unapproved dependent patch, it might be
better to hold off this discussion until either (a) that patch gets
reviewed, or (b) the approval of the patch I posted is reversed.
(It's only a suggestion -- doing patches against mainline is obviously
the right thing to do, and I'm not trying to imply otherwise.
Also, if there's something you object to in that patch -- the UNSPEC? --
then I can try to fix it.)

Richard


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