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

[Roman Zippel <zippel at linux-m68k dot org>]

Hi,

On Tue, 6 Feb 2007, Richard Sandiford wrote:

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

I misrembered this patch, I thought it was about the mov3q instruction. 
The R constraint was unused, but one of your patches activates it, so are 
you sure that ColdFire doesn't have a similiar problem now?

> >> 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.:

That one looks pretty much like in your version. :)

> 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.

Well, I don't mind making the patch incremental to yours (and my patch has 
a dependency on another patch as well), but my patch does a bit more, so 
it's not completely redundant.
I'm also thinking about other possible improvements, e.g. currently it 
converts a "pea 100" to "move.l #100,(%sp)", which would only be a win if 
the previous addq.l is eliminated, otherwise it would need a scratch 
register and a moveq to be at least the same size.

> Also, if there's something you object to in that patch -- the UNSPEC? --
> then I can try to fix it.)

The last peephole added is pretty much useless as it will never match.

bye, Roman