expand_expr tweaks to fix PR57134

[Richard Biener]

On Wed, Jun 12, 2013 at 4:48 AM, Alan Modra <amodra@gmail.com> wrote:
> The following patch fixes PR57134 by
> a) excluding bitfield expansion when EXPAND_MEMORY, and
> b) passing down the EXPAND_MEMORY modifier in a couple of places where
> this does not currently happen on recursive calls to expand_expr().
>
> (a) has precedent elsewhere in expr.c, eg. see expand_expr_real_1
> <MEM_REF>, (b) is a little difficult to justify except to claim
> there's no logical reason why it should be excluded nowadays.  Hand
> waving argument follows:
>
> Of the seven expand_expr() modifiers, recursive calls made to handle
> inner references of aggregate types currently exclude EXPAND_SUM,
> EXPAND_WRITE, and EXPAND_MEMORY from the modifier passed.  I suppose
> EXPAND_SUM is excluded so that the inner expand_expr() won't return a
> PLUS or MULT when we might be adding a further offset, but I can't see
> why the other two modifiers are not passed.  Digging through the
> history, it looks like EXPAND_WRITE may have been missed by accident,
> and EXPAND_MEMORY used to be an entirely different animal.  kenner was
> responsible for the original recursive call that passed down
> EXPAND_NORMAL, EXPAND_CONST_ADDRESS and EXPAND_INITIALIZER when expr.h
> looked like:
>
>  14612     kenner    EXPAND_MEMORY_USE_* are explained below.  */
>    406     kenner enum expand_modifier {EXPAND_NORMAL, EXPAND_SUM,
>  14612     kenner                     EXPAND_CONST_ADDRESS, EXPAND_INITIALIZER,
>  14612     kenner                     EXPAND_MEMORY_USE_WO, EXPAND_MEMORY_USE_RW,
>  14612     kenner                     EXPAND_MEMORY_USE_BAD, EXPAND_MEMORY_USE_DONT};
>    406     kenner
>  14612     kenner /* Argument for chkr_* functions.
>  14612     kenner    MEMORY_USE_RO: the pointer reads memory.
>  14612     kenner    MEMORY_USE_WO: the pointer writes to memory.
>  14612     kenner    MEMORY_USE_RW: the pointer modifies memory (ie it reads and writes). An
>  14612     kenner                   example is (*ptr)++
>  14612     kenner    MEMORY_USE_BAD: use this if you don't know the behavior of the pointer, or
>  14612     kenner                    if you know there are no pointers.  Using an INDIRECT_REF
>  14612     kenner                    with MEMORY_USE_BAD will abort.
>  14612     kenner    MEMORY_USE_TW: just test for writing, without update.  Special.
>  14612     kenner    MEMORY_USE_DONT: the memory is neither read nor written.  This is used by
>  14612     kenner                  '->' and '.'.  */
>
> So I think passing EXPAND_MEMORY and EXPAND_WRITE down ought to be
> good.  The VIEW_CONVERT_EXPR case really doesn't need to exclude
> EXPAND_SUM either, since it doesn't allow an offset, but I made it the
> same as the inner ref case so that when/if someone attends to
>         /* ??? We should work harder and deal with non-zero offsets.  */
> it doesn't cause a surprise.  Really, a lot of this code needs
> attention, for example, I think SSA made EXPAND_STACK_PARM and all of
> the related code in calls.c dealing with libcalls when expanding args,
> unnecessary.
>
> Bootstrapped and regression tested powerpc64-linux.  OK to apply?

I suppose it also fixes PR57586 which looks similar?  Can you add a testcase
please?

Ok with a testcase and mentioning PR57586 in the changelog.

Thanks,
Richard.

>         PR middle-end/57134
>         * expr.c (expand_expr_real_1 <normal_inner_ref>): Pass
>         EXPAND_MEMORY and EXPAND_WRITE to recursive call.  Don't use
>         bitfield expansion when EXPAND_MEMORY.
>         (expand_expr_real_1 <VIEW_CONVERT_EXPR>): Pass modifier likewise.
>
>
> Index: gcc/expr.c
> ===================================================================
> --- gcc/expr.c  (revision 199940)
> +++ gcc/expr.c  (working copy)
> @@ -9918,12 +9919,8 @@ expand_expr_real_1 (tree exp, rtx target, enum mac
>                           && modifier != EXPAND_STACK_PARM
>                           ? target : NULL_RTX),
>                          VOIDmode,
> -                        (modifier == EXPAND_INITIALIZER
> -                         || modifier == EXPAND_CONST_ADDRESS
> -                         || modifier == EXPAND_STACK_PARM)
> -                        ? modifier : EXPAND_NORMAL);
> +                        modifier == EXPAND_SUM ? EXPAND_NORMAL : modifier);
>
> -
>         /* If the bitfield is volatile, we want to access it in the
>            field's mode, not the computed mode.
>            If a MEM has VOIDmode (external with incomplete type),
> @@ -10081,6 +10078,7 @@ expand_expr_real_1 (tree exp, rtx target, enum mac
>                       || (MEM_P (op0)
>                           && (MEM_ALIGN (op0) < GET_MODE_ALIGNMENT (mode1)
>                               || (bitpos % GET_MODE_ALIGNMENT (mode1) != 0))))
> +                    && modifier != EXPAND_MEMORY
>                      && ((modifier == EXPAND_CONST_ADDRESS
>                           || modifier == EXPAND_INITIALIZER)
>                          ? STRICT_ALIGNMENT
> @@ -10279,10 +10277,7 @@ expand_expr_real_1 (tree exp, rtx target, enum mac
>                               && modifier != EXPAND_STACK_PARM
>                               ? target : NULL_RTX),
>                              VOIDmode,
> -                            (modifier == EXPAND_INITIALIZER
> -                             || modifier == EXPAND_CONST_ADDRESS
> -                             || modifier == EXPAND_STACK_PARM)
> -                            ? modifier : EXPAND_NORMAL);
> +                            modifier == EXPAND_SUM ? EXPAND_NORMAL : modifier);
>
>             if (MEM_P (orig_op0))
>               {
>
> --
> Alan Modra
> Australia Development Lab, IBM