Re: AM30/AM33: global simplification and standardization of patterns

[Jeffrey A Law <law at cygnus dot com>]

  In message <oraein82br.fsf@zecarneiro.lsd.ic.unicamp.br>you write:
  > --=-=-=
  > 
  > This patch factors out lots of redundant constraints in several
  > patterns scattered along mn10300.md.  In many cases, I've also
  > gratuitously reordered constraints within the same alternative so that
  > constraints always appear in the same order, and adds `*' in front of
  > `x's so as to reduce the probability of using extended registers, that
  > increase the code size.  This patch also fixes the `cc' attribute of
  > movdi and movdf insns, that can't be relied upon in any case.
  > 
  > I was glad to find out this patch considerably reduced the code size
  > on am33.  But I was sad that it increased the code size on am30 in
  > most cases.  I can't explain why.  Can anybody?
Over-zealous gratuitous changes and trying to change too many things at
once.  I would have broken this up into smaller patches which might have
made explaining the increase easier.

For example I would have gone after the integer moves as one hunk, then the
FP moves, then the comparisons, then arithmetic, then logicals, then shifts,
etc.

Sounds like more work and it is.  But you would have a much better idea of
what change is causing your code size increases than you do right now.

  > 	* config/mn10300/mn10300.md (movqi, movhi, movsi, movsf): Simplify.
  > 	Prefer non-extended registers.
  > 	(movdi, movdf): Likewise.  Fix `cc' attributes.  Use AM33 regs.
  > 	(tstsi, cmpsi, addsi): Prefer non-extended registers.
  > 	(subsi, mulsidi3, umulsidi3, udivmodsi4, andsi3, iorsi3, xorsi3):
  > 	Reordered constraints.  Prefer non-extended registers.
  > 	(one_cmplsi2, bclr, bset, zero_extendqisi2, zero_extendhisi2,
  > 	extendqisi2, extendhisi2, ashlsi3, lshrsi3, ashrsi3): Prefer
  > 	non-extended registers.
Given the size regressions, no this is not ok.

Now since we're looking for size regressions on the mn103/am30 port we know
that insns which are dependent on TARGET_AM33 can't be the cause.  That helps
narrow things down a little.

Each change you make should have a purpose -- reordering for just reordering's
sake is not a good idea.  Sometimes order matters, though I don't remember
when off the top of my head.

It also isn't necessarily a good idea to combine alternatives like you have;
I believe that can make a difference in register class preferencing, though
I'm not 100% sure.

  >  (define_insn ""
  > -  [(set (match_operand:QI 0 "general_operand" "=dx,*a,dx,*a,dx,*a,dx,*a,dx
  > ,m")
  > -	(match_operand:QI 1 "general_operand" "0,0,I,I,a,dx,dxi,ia,m,dx"))]
  > +  [(set (match_operand:QI 0 "general_operand" "=d*a,d,da,d,m")
  > +	(match_operand:QI 1 "general_operand" "0,I,dai,m,d"))]
You can no longer clear an address register effectively after this change.
That may be part of the problem.  You did the same thing to several other
patterns.


  >  (define_insn ""
  > -  [(set (match_operand:HI 0 "general_operand" "=dx,*a,dx,*a,dx,*a,dx,*a,dx
  > ,m")
  > -	(match_operand:HI 1 "general_operand" "0,0,I,I,a,dx,dxi,ia,m,dx"))]
  > +  [(set (match_operand:HI 0 "general_operand" "=d*a,d,da,d,m")
  > +	(match_operand:HI 1 "general_operand" "0,I,dai,m,d"))]
Similarly.

  > @@ -248,7 +228,7 @@
  >  (define_expand "reload_insi"
  >    [(set (match_operand:SI 0 "register_operand" "=a")
  >  	(match_operand:SI 1 "impossible_plus_operand" ""))
  > -   (clobber (match_operand:SI 2 "register_operand" "=&r"))]
  > +   (clobber (match_operand:SI 2 "register_operand" "=&da"))]
"r" was used here for a reason.  Constraints on the reload_XXX patterns
have special meanings, read the section on secondary reloads in the
GCC manual.  This might also be affecting code size on the mn103/am30.


There may be other problems with this patch.  I didn't look at it in much
detail since it needs a number of changes and to be broken up into manageable
hunks before we can really review and install it.

jeff