On Mon, Feb 18, 2019 at 08:40:12AM -0600, Matthew Malcomson wrote: > Handle stack pointer with SUBS/ADDS instructions. > > In general the stack pointer was not handled for many SUBS/ADDS patterns in > aarch64.md. > Both the "extended register" and "immediate" forms allow the stack pointer to > be > used as the source register, while no form allows the stack pointer for the > destination register. > > The define_insn patterns generating ADDS/SUBS did not allow the stack pointer > for any operand, while the define_peephole2 patterns that generated RTX to be > matched by these patterns allowed the stack pointer for any operand. > > The patterns are fixed by adding the 'k' constraint for the first source > operand > to all define_insns that generate the ADDS/SUBS "extended register" and > "immediate" forms (but not the "shifted register" form). > > In peephole optimizations, constraint strings are ignored (see "(gccint) C > Constraint Interface" info node in the documentation), so the decision to act > or > not is based solely on the predicate and condition. > This patch introduces a new predicate "aarch64_general_reg" to be used in > define_peephole2 patterns where only GENERAL_REGS registers are acceptable and > uses that predicate in the peepholes that generate patterns for ADDS/SUBS. > > Additionally, this patch contains two tidy-ups (happy to remove them or put in > a separate patch if people want):
Generally, yes I prefer that. > We change the condition of sub<mode>3_compare1_imm pattern from checking > "UINTVAL (operands[2]) == -UINTVAL (operands[3])" > to checking > "INTVAL (operands[2]) == -INTVAL (operands[3])" > for clarity, since the values checked are signed integers, there are negations > involved in the check, and the condition used by the corresponding peepholes > also uses INTVAL. > > The superfluous <w> iterator in the assembly template for > add<mode>3_compareV_imm is removed -- it was applied to an operand that is > known to be a const_int. > > Full bootstrap and regtest done on aarch64-none-linux-gnu. > Regression tests done on aarch64-none-linux-gnu and aarch64-none-elf cross > compiler. > > OK for trunk? This patch is fine for GCC 10, so not on trunk yet please unless there is a corectness reason for the change. > NOTE: I have included a bunch of RTL testcases that I used in development, > these > don't exercise much of the compiler and are pretty specific to the backend as > it > currently is, so I'm not sure they give much value. I'd appreciate feedback on > whether this is in general considered useful. I would happily take the RTL test, do you want to check with a testsuite maintainer? > gcc/ChangeLog: > > 2019-02-18 Matthew Malcomson <matthew.malcom...@arm.com> > > PR target/89324 > * config/aarch64/aarch64.md: Use aarch64_general_reg predicate on > destination register in peepholes generating patterns for ADDS/SUBS. > (add<mode>3_compare0, > *addsi3_compare0_uxtw, add<mode>3_compareC, > add<mode>3_compareV_imm, add<mode>3_compareV, > *adds_<optab><ALLX:mode>_<GPI:mode>, > *subs_<optab><ALLX:mode>_<GPI:mode>, > *adds_<optab><ALLX:mode>_shift_<GPI:mode>, > *subs_<optab><ALLX:mode>_shift_<GPI:mode>, > *adds_<optab><mode>_multp2, *subs_<optab><mode>_multp2, > *sub<mode>3_compare0, *subsi3_compare0_uxtw, > sub<mode>3_compare1): Allow stack pointer for source register. > * config/aarch64/predicates.md (aarch64_general_reg): New predicate. > > > gcc/testsuite/ChangeLog: > > 2019-02-18 Matthew Malcomson <matthew.malcom...@arm.com> > > PR target/89324 > * gcc.dg/rtl/aarch64/subs_adds_sp.c: New test. > * gfortran.fortran-torture/compile/pr89324.f90: New test. > > > > ############### Attachment also inlined for ease of reply > ############### I appreciate that. Thanks, James
