RL78 one_cmplhi2 improvement

Hello,

The following patch defines one_cmplhi2 pattern.  The improvement does not
come from the two xor instructions used 
(in fact for the second xor the pattern uses xor saddr , #byte is used which
is 1 bytes longer than xor a, #byte) it comes 
from the fact that that the number of move instructions is reduced (16 bit
movw is used instead of 8 bit mov).

This can be seen on a very simple test case:

uint16_t test_one_cmplhi(uint16_t x) 
{
 return ~x; 
}

_test_one_cmplhi:
	mov	a, [sp+4]
	xor	a, #-1
	mov	r8, a
	mov	a, [sp+5]
	xor	a, #-1
	mov	r9, a
	ret

_test_one_cmplhi:
	movw	ax, [sp+4]
	xor a, #-1 
	xor 0xFFEF8, #-1 ;one_cmplhi2 ax, ax
	movw	r8, ax
	ret

In "gcc.c-torture/execute/" I have seen the patch being effective in 18
cases with the biggest improvement is occurring in
gcc.c-torture/execute/pr68376-2 where this patch reduces the code size from
792 bytes to 704.

Unfortunately there's also a downside to this patch. In cases when the
second xor is redundant GCC never gets the chance to remove the second xor. 
This can be seen in gcc.c-torture/execute/pr42833.c for example line 57: 
vdest.v1 = (vsrc1.v1 + (1 << (-1 - tmp))) >> -tmp;
This is because all variables are 8 bit while the operation is being
promoted to 16 bit (because of the constants being of type int by default)
this is why the one_cmplhi2 is used in this case.

The relevant insns for this case are:

(insn 35 257 260 14 (set (reg:HI 0 x)
        (xor:HI (reg:HI 0 x)
            (const_int -1 [0xffffffffffffffff])))
"./xgcc_rw_trunk2/gcc/testsuite/gcc.c-torture/execute/pr42833.c":57 59
{*one_cmplhi2_real}
     (nil))
(insn 260 35 261 14 (set (reg:QI 1 a)
        (reg:QI 0 x [8]))
"./xgcc_rw_trunk2/gcc/testsuite/gcc.c-torture/execute/pr42833.c":57 44
{*movqi_real}
     (expr_list:REG_DEAD (reg:QI 8 r8)
        (nil)))

As it can be seen it is quite obvious that reg "a" is dead after insn 35.
I think I can write a simple rtl_opt_pass (similar to pass_rl78_move_elim)
to look for such cases and replace the one_cmplhi2 with one_cmplqi2.

Even without this new pass (which I plan to do in the near future) I still
think (based on testing) the advantages of this patch outweigh the
disadvantage described above.
Is OK to check-in? Or do I need to add the new pass as well for this to be
considered? Thank you!

Regression test is OK, tested with the following command:
make -k check-gcc RUNTESTFLAGS=--target_board=rl78-sim


Best Regards,
Sebastian

2018-02-20  Sebastian Perta  <sebastian.perta@renesas.com>

	* config/rl78/rl78-expand.md (one_cmplhi2): New define expand.
	* config/rl78/rl78-virt.md (one_cmplhi2_virt): New define insn.
	* config/rl78/rl78-real.md (one_cmplhi2_real): New define insn.

HI DJ,

> One thing to try is to use (subreg:QI in a define_expand, so that
> there's a one_cmplhi2 pattern that expands to two QImode insns that
> operate on HImode input/outputs via SUBREGs.

Thank you for the suggestion! After several attempts the following is the
only successful one, however the code produced is identical with and without
the patch:

(define_expand "one_cmplhi2"
 [(set (subreg:QI (match_operand:HI 0 "nonimmediate_operand") 0)
  (xor:HI (subreg:QI (match_operand:HI 1 "general_operand") 0)
  		(const_int -1)))
  (set (subreg:QI (match_dup 0) 1)
  (xor:HI (subreg:QI (match_dup 1) 1)
  		(const_int -1)))
  ]
  ""
  "DONE;"
)

Is this similar to what you had in mind?

Output code (same as before the patch ... the patch makes no difference):
_test_one_cmplhi:
	mov	a, [sp+4]
	xor	a, #-1
	mov	r8, a
	mov	a, [sp+5]
	xor	a, #-1
	mov	r9, a
	ret

I also explored other options including define_split without any success.

> If it doesn't work out, consider this patch approved, though.
Can I checkin now?

Best Regards,
Sebastian


> -----Original Message-----
> From: DJ Delorie [mailto:dj@redhat.com]
> Sent: 20 February 2018 19:39
> To: Sebastian Perta <Sebastian.Perta@renesas.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] RL78 one_cmplhi2 improvement
> 
> 
> Const type promotion is the bane of embedded developers...
> 
> One thing to try is to use (subreg:QI in a define_expand, so that
> there's a one_cmplhi2 pattern that expands to two QImode insns that
> operate on HImode input/outputs via SUBREGs.
> 
> I don't have high hopes of gcc optimizing this properly in all cases,
> but it's worth trying.
> 
> If it doesn't work out, consider this patch approved, though.
> 
> Thanks!