Oleg Endo <[email protected]> wrote:
> The attached patch improves the generated code for integer abs
> operations on SH, in particular SH4. There was already some code that
> was supposed to utilize SH's conditional execution it but it was never
> triggered, because the standard branch-free abs code was generated at a
> very early stage.
> Since the DI mode part of the original patch is causing problems I've
> stripped it out for now.
>
> Tested with make -k check RUNTESTFLAGS="--target_board=sh-sim
> \{-m2,-m2a-single,-m4-single,-m4a-single\}\{-mb,-ml\}" with no new
> failures.
>
> CSiBE code size tests show small decreases in a couple of places, with
> the highest drop being in libpng-1.2.5,pngrutil (15800 -> 15712) for
> -m4-single.
Thanks for this work! A few minor style issues:
> * config/sh/sh.md (negdi2): Moved expansion into split to
> allow more combination options. Added T_REG clobber.
> (*negdi2, abssi2, *abssi2, *negabssi2): Added.
> (cneg): Changed from insn to insn_and_split. Renamed to
> negsi_cond. Added alternative for non-SH4.
Please add PR target/49468 at just before this entry like as
the other entries which are tied with the PR. The usual form
for ChangeLog entries is as a log of changes in imperative form.
For new insns, "New insns." is a usual way of gcc ChangeLog.
See other similar entries as examples.
>+(define_insn_and_split "negsi_cond"
>+ [(set (match_operand:SI 0 "arith_reg_dest" "=r,r")
>+ (if_then_else:SI (eq:SI (reg:SI T_REG)
>+ (match_operand:SI 3
>"const_int_operand" "M,N"))
[snip]
>+ emit_jump_insn (INTVAL (operands[3])
>+ ? gen_branch_true (skip_neg_label)
>+ : gen_branch_false (skip_neg_label));
>+
>+ emit_label_after (skip_neg_label,
>+ emit_insn (gen_negsi2
>(operands[0], operands[1])));
Some lines of the patch look too long. Use 8 spaces wide tabs
for GCC patches as GNU/GCC coding style says.
OK with those style changes.
Regards,
kaz
