Re: [patch] expr.c: Fix the failure of gcc.dg/compat/scalar-by-value-4.

[Jim Wilson <wilson at tuliptree dot org>]

Kazu Hirata wrote:
, stack_pointer_rtx);
> +  /* If we are to pad after the pushed object, adjust the stack
> +     pointer now and store X into the stack location using an offset.
> +     This is because emit_move_insn does not know how to pad; it does
> +     not have access to type.  */
> +  else if (FUNCTION_ARG_PADDING (mode, type) == stack_direction)

I think this test is wrong.  If arg and stack padding are both upward, 
then we want to emit 2 bytes of operand followed by 2 bytes of padding. 
 There is no special offset needed here, since this is just a 2 byte 
store followed by a 4 byte increment.

I think the problem case is always when FUNCTION_ARG_PADDING is 
downward.  In that case, we are always writing into the middle of the 
new stack area.  Does this sound right to you?

I also think there are latent bugs here in the !STACK_GROWS_DOWNWARD 
case.  The code is using PRE_MODIFY and PRE_INC for a stack that grows 
upward.  This is wrong.  It should be POST_MODIFY/POST_INC.  For a 
downward growing stack, a push is a PRE_DEC and a pop is a POST_INC. 
For an upward growing stack, a push is a POST_INC, and a pop is a 
PRE_DEC.  Fixing this would require changes to recog.c and changes to 
some targets (e.g. c4x).  A quick grep shows 5 targets that don't define 
STACK_GROWS_DOWNWARD.  Four of them define ACCUMULATE_OUTGOING_ARGS, and 
the other one has explicit push patterns, so none of them use the code 
here.  I don't like "fixing" code without testcases, but I think we 
should at least document this with a ??? comment.

The rest of the patch looks OK.

Jim