Message ID | 20130612024834.GY6878@bubble.grove.modra.org |
---|---|
State | New |
Headers | show |
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
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)) {