On 7/9/24 4:03 AM, Georg-Johann Lay wrote:
Hi Jeff,
This patch adds peephole2s and insns to make better use of
instructions that set condition code (SREG) as a byproduct.
Of course with cc0 all this was *much* simpler... so here we go;
adding CCNmode and CCZNmode, and extra insns that do arith + CC.
No new regressions.
Ok for master?
Johann
--
AVR: target/115830 - Make better use of SREG.N and SREG.Z.
This patch adds new CC modes CCN and CCZN for operations that
set SREG.N, resp. SREG.Z and SREG.N. Add a bunch of peephole2
patterns to generate new compute + branch insns that make use
of the Z and N flags. Most of these patterns need their own
asm output routines that don't do all the micro-optimizations
that the ordinary outputs may perform, as the latter have no
requirement to set CC in a usable way. Pass peephole2 is run
a second time so all patterns get a chance to match.
PR target/115830
gcc/
* config/avr/avr-modes.def (CCN, CCZN): New CC_MODEs.
* config/avr/avr-protos.h (ret_cond_branch): Adjust.
(avr_out_plus_set_N, avr_op8_ZN_operator,
avr_out_op8_set_ZN, avr_len_op8_set_ZN): New protos.
* config/avr/avr.cc (ret_cond_branch): Remove "reverse"
argument (was always false) and respective code.
Pass cc_overflow_unusable as an argument.
(cond_string): Add bool cc_overflow_unusable argument.
(avr_print_operand) ['L']: Like 'j' but overflow unusable.
['K']: Like 'k' but overflow unusable.
(avr_out_plus_set_ZN): Also support adding -2 and +2.
(avr_out_plus_set_N, avr_op8_ZN_operator): New functions.
(avr_out_op8_set_ZN, avr_len_op8_set_ZN): New functions.
(avr_adjust_insn_length) [ADJUST_LEN_ADD_SET_N]: Hande case.
(avr_class_max_nregs): All MODE_CCs occupy one hard reg.
(avr_hard_regno_nregs): Same.
(avr_hard_regno_mode_ok) [REG_CC]: Allow all MODE_CC.
(pass_manager.h): Include it.
(avr_option_override): Run peephole2 a second time.
* config/avr/avr.md (adjust_len) [add_set_N]: New.
(ALLCC, CCN_CCZN): New mode iterators.
(CCname): New mode attribute.
(eqnegtle, cmp_signed, op8_ZN): New code iterators.
(swap, SWAP, tstMSB): New code attributes.
(branch): Handle CCNmode and CCZNmode. Assimilate...
(difficult_branch): ...this insn.
(p1m1): Turn into p2m2.
(gen_add_for_<code>_<mode>): Adjust to CCNmode and CCZNmode.
Extend peephole2s that produce them.
(*add.for.eqne.<mode>): Extend to
*add.for.<CCN_CCZN:mode>.<QISI:mode>.
(*ashift.for.ccn.<mode>): New insns and peephole2s to make them.
(*op8.for.cczn.<code>): New insns and peephole2s to make them.
* config/avr/predicates.md (const_1_to_3_operand)
(abs1_abs2_operand, signed_comparison_operator)
(op8_ZN_operator): New predicates.
gcc/testsuite/
* gcc.target/avr/pr115830-add-c.c: New test.
* gcc.target/avr/pr115830-add-i.c: New test.
* gcc.target/avr/pr115830-and.c: New test.
* gcc.target/avr/pr115830-asl.c: New test.
* gcc.target/avr/pr115830-asr.c: New test.
* gcc.target/avr/pr115830-ior.c: New test.
* gcc.target/avr/pr115830-lsr.c: New test.
* gcc.target/avr/pr115830-asl32.c: New test.
I was going to throw this into my tester, but the avr.md part of the
patch failed. I'm guessing the patch needs minor updates due to some
kind of changes on the trunk.
It looks like avr exposes the CC register early, creating references to
it during expansion to RTL. Presumably this means you've got a
reasonbale way to reload values, particularly address arithmetic without
impacting the CC state?
It looks like you're relying heavily on peep2 patterns. Did you explore
using cmpelim?
pr115830-1.diff
diff --git a/gcc/config/avr/avr-modes.def b/gcc/config/avr/avr-modes.def
index e0633d680d5..77b05dd8509 100644
--- a/gcc/config/avr/avr-modes.def
+++ b/gcc/config/avr/avr-modes.def
@@ -18,6 +18,11 @@
FRACTIONAL_INT_MODE (PSI, 24, 3);
+/* Used when the N (and Z) flag(s) of SREG are set.
+ The N flag indicates whether the value is negative. */
+CC_MODE (CCN);
+CC_MODE (CCZN);
N and Z are probably the easiest to model. C is usually worth modeling
as well. But obviously that could be a follow-up.
+ if (ival >= +1) avr_asm_len ("inc %0", xop, plen, 1);
+ if (ival == +2) avr_asm_len ("inc %0", xop, plen, 1);
+ if (ival <= -1) avr_asm_len ("dec %0", xop, plen, 1);
+ if (ival == -2) avr_asm_len ("dec %0", xop, plen, 1);
Formatting issue. Bring the avr_asm_len calls down to their own lines,
indented appropriately. However, if this is a common style in avr.cc,
particularly in that same function then it's OK to leave as-is.
Not really ready to ACK or NAK at this point.
jeff
