Re: [PATCH] AVR: implement HI/SI logic operations and sign/zero extension by define_insn_and_split instead of define_insn

[Denis Chertykov <denisc at overta dot ru>]

Björn Haase <bjoern.m.haase@web.de> writes:

> Hi,
> 
> here is the first result of my work on the "splitting" issue for AVR.
> Find enclosed a patch that passed my testsuite run without any regression.

I have reviewed your patch.

1. Significant part of goal of rewriting this code was to remove
   output templates from HI and SI mode of logic insns.
IE:
(define_insn_and_split "iorhi3"
  [(set (match_operand:HI 0 "register_operand" "=r,d")
	(ior:HI (match_operand:HI 1 "register_operand" "%0,0")
		(match_operand:HI 2 "nonmemory_operand" "r,i")))]
  ""
  "*{
  if (which_alternative==0)
    return (AS2 (or,%A0,%A2) CR_TAB
	    AS2 (or,%B0,%B2));
   ...
   ...
   }")

Must be changed to:

(define_insn_and_split "iorhi3"
  [(set (match_operand:HI 0 "register_operand" "=r,d")
	(ior:HI (match_operand:HI 1 "register_operand" "%0,0")
		(match_operand:HI 2 "nonmemory_operand" "r,i")))]
  ""
  "#"
   ...
   ...)




2. The following code in your patterns seems strange:
  "if (GET_CODE(operands[2]) == CONST_INT)
    {
      /* If operands[2] is a register, use the template above.  */ 
      int value = INTVAL(operands[2]);
      int j;
      int bytes_of_mode = 2;
      for (j=0; j < bytes_of_mode; j++)
   ...
   ..."

Look at ARM port:

;; Boolean and,ior,xor insns

;; Split up double word logical operations

;; Split up simple DImode logical operations.  Simply perform the logical
;; operation on the upper and lower halves of the registers.
(define_split
  [(set (match_operand:DI 0 "s_register_operand" "")
	(match_operator:DI 6 "logical_binary_operator"
	  [(match_operand:DI 1 "s_register_operand" "")
	   (match_operand:DI 2 "s_register_operand" "")]))]
  "TARGET_ARM && reload_completed && ! IS_IWMMXT_REGNUM (REGNO (operands[0]))"
  [(set (match_dup 0) (match_op_dup:SI 6 [(match_dup 1) (match_dup 2)]))
   (set (match_dup 3) (match_op_dup:SI 6 [(match_dup 4) (match_dup 5)]))]
  "
  {
    operands[3] = gen_highpart (SImode, operands[0]);
    operands[0] = gen_lowpart (SImode, operands[0]);
    operands[4] = gen_highpart (SImode, operands[1]);
    operands[1] = gen_lowpart (SImode, operands[1]);
    operands[5] = gen_highpart (SImode, operands[2]);
    operands[2] = gen_lowpart (SImode, operands[2]);
  }"
)

gen_lowpart and gen_highpart will work right with constant operand.

Denis.