https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79514
--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> --- (In reply to Uroš Bizjak from comment #10) > (In reply to Jakub Jelinek from comment #9) > Years ago, I defined HAVE_PRE_DEC, and I was told that it is intended for > targets that can increment pointer on *any* memory access, not just on stack > pointer. So, x86 uses PRE_DEC and POST_INC only to abstract push and pop > insn. Various parts of the compiler (modulo auto-inc-dec.c) mostly check for > PRE_DEC and POST_INC to skip further optimizations, which is non-issue for > push and pop. > > But this generic part of the code assumes that any target can handle > PRE_MODIFY, and emits insn even for !HAVE_PRE_MODIFY_DISP. This is > conservatively incorrect, so conditionally disabling the part that blindly > emits PRE_MODIFY is IMO the way to go. But isn't that the same? Targets can only define HAVE_PRE_MODIFY_DISP, if they support PRE_MODIFY with arbitrary registers, but when they only support it with the stack pointer, they can't. > Maybe this issue should be discussed on the ML. Maybe. This matters only for targets that define PUSH_ROUNDING and define it to something other than unconditionally the first argument. I think that is cr16, h8300, i386, m32c, m68k, pdp11, sh, stormy16 None of these targets define HAVE_PRE_MODIFY_DISP, though h8300 clearly has patterns with pre_modify on stack_pointer_rtx (thus relies on the patch I've attached not to be applied), and i386.c, m32c.c, pdp11.c stormy16.c all have PRE_MODIFY tests at least in one place, e.g. pdp11.c has: /* accept -(SP) -- which uses PRE_MODIFY for byte mode */ comment (and code inspection agrees with that), so at least pdp11 and h8300 would be broken by that change, maybe m32c and stormy16 too. > BTW: > > - dest_addr = gen_rtx_PLUS (Pmode, stack_pointer_rtx, > - gen_int_mode (offset, Pmode)); > + if (offset) > + dest_addr = gen_rtx_PLUS (Pmode, stack_pointer_rtx, > + gen_int_mode (offset, Pmode)); > + else > + dest_addr = stack_pointer_rtx; > > plus_constant? Sure, iff we want to do that change.
