gcc-bugs@gcc.gnu.org

[Toshi Morita <tm2 at best dot com>]

This patch:

Wed Mar 21 10:25:13 CET 2001  Jan Hubicka  <jh@suse.cz>

        * i386.md (pophi1, popqi1, pushqi1): Remove.

        * expr.c (emit_single_push_insn): New function.
        (move_by_pieces): Accept NULL as destination for push instructions.
        (gen_push_operand): Kill.
        (X connection to :0.0 broken (explicit kill or server shutdown).
        stack_pointer_delta.
        * expr.h (gen_push_operand): Kill.

...seems to be causing --target=h8300-coff build failures.

Specifically, it adds a new function emit_single_push_insn, which contains
this code:

#ifdef STACK_GROWS_DOWNWARD
      dest_addr = gen_rtx_PLUS (Pmode, stack_pointer_rtx,
                                GEN_INT (-rounded_size));
#else
      dest_addr = gen_rtx_PLUS (Pmode, stack_pointer_rtx,
                                GEN_INT (rounded_size));
#endif
      dest_addr = gen_rtx_PRE_MODIFY (Pmode, stack_pointer_rtx, dest_addr);
    }

  dest = gen_rtx_MEM (mode, dest_addr);

  stack_pointer_delta += PUSH_ROUNDING (GET_MODE_SIZE (mode)); *** ??? ***

  if (type != 0)
    {
      set_mem_attributes (dest, type, 1);
      /* Function incoming arguments may overlap with sibling call
         outgoing arguments and we cannot allow reordering of reads
         from function arguments with stores to outgoing arguments
         of sibling calls.  */
      MEM_ALIAS_SET (dest) = 0;
    }

It appears wrong to me that emit_single_push_insn is modifying
stack_pointer_delta, because this is supposed to be done by emit_move_insn_1.

In the case of the H8300, this adjustment of stack_pointer_delta breaks
the target build, because the stack pointer winds up being adjust twice:
once by emit_single_push_insn, and then later by emit_move_insn_1 when
it calls anti_adjust_stack.

I believe the correct place to apply the stack adjustment for pushes is in
emit_move_insn_1. There appears to be a broken corner case where it fails
to call anti_stack_adjust to adjust the stack_pointer_delta:

rtx
emit_move_insn_1 (x, y)
     rtx x, y;
{
  enum machine_mode mode = GET_MODE (x);
  enum machine_mode submode;
  enum mode_class class = GET_MODE_CLASS (mode);
  unsigned int i;

  if ((unsigned int) mode >= (unsigned int) MAX_MACHINE_MODE)
    abort ();

  if (mov_optab->handlers[(int) mode].insn_code != CODE_FOR_nothing) 
    return
      emit_insn (GEN_FCN (mov_optab->handlers[(int) mode].insn_code) (x, y)); <- HERE

I believe this code should be changed to look something like:

  if (mov_optab->handlers[(int) mode].insn_code != CODE_FOR_nothing)
    {
      /* More code needed here to handle PUSH_ROUNDING... */
      if (push_operand (x, GET_MODE (x)))
        {
          anti_adjust_stack (GEN_INT (GET_MODE_SIZE (GET_MODE (x))));
        }
      return emit_insn (GEN_FCN (mov_optab->handlers[(int) mode].insn_code) (x, y));
    }

Toshi