Hi Richard, "Richard Earnshaw (lists)" <richard.earns...@arm.com> writes:
>> gcc/ChangeLog: >> 2020-??-?? Andrea Corallo <andrea.cora...@arm.com> >> 2020-??-?? Mihail-Calin Ionescu <mihail.ione...@arm.com> >> 2020-??-?? Iain Apreotesei <iain.apreote...@arm.com> >> * config/arm/arm.c (TARGET_INVALID_WITHIN_DOLOOP): >> (arm_invalid_within_doloop): Implement invalid_within_doloop hook. >> * config/arm/arm.h (TARGET_HAVE_LOB): Add new macro. >> * config/arm/thumb2.md (*doloop_end, doloop_begin, dls_insn): >> Add new patterns. >> * config/arm/unspecs.md: Add new unspec. >> > > A date should only appear before the first author in a multi-author > patch, other authors should then be indented to align with the name of > that first author. Ack > +(define_insn "*doloop_end" > + [(parallel [(set (pc) > + (if_then_else > + (ne (reg:SI LR_REGNUM) (const_int 1)) > + (label_ref (match_operand 0 "" "")) > + (pc))) > + (set (reg:SI LR_REGNUM) > + (plus:SI (reg:SI LR_REGNUM) (const_int -1)))])] > + "TARGET_32BIT && TARGET_HAVE_LOB && !flag_modulo_sched" > + "le\tlr, %l0") > > Is it deliberate that this pattern name has a '*' prefix? doloop_end > is a named expansion pattern according to md.texi. Yes, this should be expanded already by the define_expand we have in thumb2.md. Perhaps I'll call it 'doloop_end_internal' and add a comment. > Also, hard-coded register names should be prefixed with '%|' (so > "%|lr", not just "lr"), just in case the assembler dialect requires > something (ELF doesn't but others have). Also for dls_insn. Ack > For the tests, your 'require-effective-taret' tests look insufficient > to prevent problems when testing a multilib environment, you'll need > (at least) checks that a) passing -marm has not happened and b) that > the architecture, or a specific CPU isn't being passed on the command > line. Ack Thanks for reviewing I'll update the patch. Andrea
