i386 codegen tweek 2

Jan Hubicka jh@suse.cz
Tue Jun 11 10:14:00 GMT 2002


> On Tue, Jun 11, 2002 at 04:10:56PM +0200, Jan Hubicka wrote:
> > + (define_insn "*addqi_1_slp"
> > +   [(set (strict_low_part (match_operand:QI 0 "nonimmediate_operand" "+qm,q"))
> > + 	(plus:QI (match_operand:QI 1 "nonimmediate_operand" "%0,0")
> > + 		 (match_operand:QI 2 "general_operand" "qn,qnm")))
> > +    (clobber (reg:CC 17))]
> > +   "TARGET_PARTIAL_REG_STALL
> > +    && ix86_binary_operator_ok (PLUS, QImode, operands)"
> 
> Don't you mean !TARGET_PARTIAL_REG_STALL?

Oops, deifnitly.
> 
> > + {
> > +   /* Make things pretty and `subl $4,%eax' rather than `addl $-4, %eax'.
> > +      Exceptions: -128 encodes smaller than 128, so swap sign and op.  */
> > +   if (GET_CODE (operands[2]) == CONST_INT
> > +       && INTVAL (operands[2]) < 0)
> > +     {
> > +       operands[2] = GEN_INT (-INTVAL (operands[2]));
> > +       return "sub{b}\t{%2, %0|%0, %2}";
> > +     }
> > +   return "add{b}\t{%2, %0|%0, %2}";
> > + }
> > +   [(set (attr "type")
> > +      (if_then_else (match_operand:QI 2 "incdec_operand" "")
> > + 	(const_string "incdec")
> > + 	(const_string "alu")))
> 
> You advertize incdec, but don't implement them.
> Comment is incorrect for -128.

I see I was somewhat overactive about zapping the code, thanks!
> 
> > + (define_insn "*subqi_1_slp"
> > +   [(set (strict_low_part (match_operand:QI 0 "nonimmediate_operand" "+qm,q"))
> > + 	(minus:QI (match_operand:QI 1 "nonimmediate_operand" "0,0")
> > + 		  (match_operand:QI 2 "general_operand" "qn,qmn")))
> > +    (clobber (reg:CC 17))]
> > +   "ix86_binary_operator_ok (MINUS, QImode, operands)"
> 
> No TARGET_PARTIAL_REG_STALL check.
> 
> > + (define_insn "iorqi_ext_0"
> > +   [(set (zero_extract:SI (match_operand 0 "ext_register_operand" "=Q")
> > + 			 (const_int 8)
> > + 			 (const_int 8))
> > + 	(ior:SI 
> > + 	  (zero_extract:SI
> > + 	    (match_operand 1 "ext_register_operand" "0")
> > + 	    (const_int 8)
> > + 	    (const_int 8))
> > + 	  (match_operand 2 "const_int_operand" "n")))
> > +    (clobber (reg:CC 17))]
> > +   ""
> 
> Again, surely we want TARGET_PARTIAL_REG_STALL on all these ext
> patterns.  You'd need to benchmark on P2/P3 to convince me it's
> safe to enable them.

It is definitly unsafe.  The partial_reg_stall check is not used in the
existing patterns as well.
Would be OK to do this in seaprate patch (disable all the patterns?)

I would guess there to be problems with splitters/peep2s using them and
I don't want ot get into snowball effect.
> 
> What sort of code size and or performance improvement do you get
> for this on Athlon (which is where I assume you tested it)?

I was getting 5KB differnce on the eon (spec2000 benchmark) binary.
THis may be special case, since it does a lot of operations on flags
that translates into and/ors with short immediates, but thats the case
of other programs as well.

Honza
> 
> 
> r~



More information about the Gcc-patches mailing list