Re: expand_expr tweaks to fix PR57134
I'm committing this cleanup patch to my PR 57134,57586 changes as
obvious. That it is obvious can be seen from an assert in
tree-ssa-operands.c get_asm_expr_operands().
/* This should have been split in gimplify_asm_expr. */
gcc_assert (!allows_reg || !is_inout);
Bootstrapped, etc. powerpc64-linux.
* stmt.c (expand_asm_operands): Revert part of 2013-09-24 special
casing inout operands.
Index: gcc/stmt.c
===
--- gcc/stmt.c (revision 203053)
+++ gcc/stmt.c (working copy)
@@ -807,9 +807,7 @@ expand_asm_operands (tree string, tree outputs, tr
|| is_inout)
{
op = expand_expr (val, NULL_RTX, VOIDmode,
- !allows_reg ? EXPAND_MEMORY
- : !is_inout ? EXPAND_WRITE
- : EXPAND_NORMAL);
+ !allows_reg ? EXPAND_MEMORY : EXPAND_WRITE);
if (MEM_P (op))
op = validize_mem (op);
--
Alan Modra
Australia Development Lab, IBM
Re: expand_expr tweaks to fix PR57134
On Fri, Sep 13, 2013 at 12:37:20PM +0930, Alan Modra wrote: PR middle-end/57586 * stmt.c (expand_asm_operands): Call expand_expr with EXPAND_MEMORY for output operands that disallow regs. Don't use EXPAND_WRITE on inout operands. Ping? -- Alan Modra Australia Development Lab, IBM
Re: expand_expr tweaks to fix PR57134
On Tue, Sep 24, 2013 at 12:43 PM, Alan Modra amo...@gmail.com wrote: On Fri, Sep 13, 2013 at 12:37:20PM +0930, Alan Modra wrote: PR middle-end/57586 * stmt.c (expand_asm_operands): Call expand_expr with EXPAND_MEMORY for output operands that disallow regs. Don't use EXPAND_WRITE on inout operands. Ping? Ok. Thanks, Richard. -- Alan Modra Australia Development Lab, IBM
Re: expand_expr tweaks to fix PR57134
This is a followup to
http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00837.html
which is still lacking an OK. Apologies for dropping this patch on
the floor.
PR middle-end/57586
* stmt.c (expand_asm_operands): Call expand_expr with
EXPAND_MEMORY for output operands that disallow regs. Don't
use EXPAND_WRITE on inout operands.
Index: gcc/stmt.c
===
--- gcc/stmt.c (revision 202428)
+++ gcc/stmt.c (working copy)
@@ -806,7 +806,10 @@ expand_asm_operands (tree string, tree outputs, tr
|| ! allows_reg
|| is_inout)
{
- op = expand_expr (val, NULL_RTX, VOIDmode, EXPAND_WRITE);
+ op = expand_expr (val, NULL_RTX, VOIDmode,
+ !allows_reg ? EXPAND_MEMORY
+ : !is_inout ? EXPAND_WRITE
+ : EXPAND_NORMAL);
if (MEM_P (op))
op = validize_mem (op);
On Fri, Jun 14, 2013 at 11:38:58AM +0200, Richard Biener wrote:
On Fri, Jun 14, 2013 at 10:38 AM, Alan Modra amo...@gmail.com wrote:
On Thu, Jun 13, 2013 at 10:45:38AM +0200, Richard Biener wrote:
On Wed, Jun 12, 2013 at 4:48 AM, Alan Modra amo...@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().
I suppose it also fixes PR57586 which looks similar?
Not completely. It cures the ICE, but you still get output number 0
not directly addressable. The reason being that expand_asm_operands
isn't asking for a mem. It should I guess, and also not specify
EXPAND_WRITE on an inout parameter. Bootstrapped and regression
tested powerpc64-linux.
It looks reasonable to me, but I'm not too familiar with EXPAND_MEMORY
vs. EXPAND_WRITE.
For the expr.h comment
EXPAND_WRITE means we are only going to write to the resulting rtx.
So fairly obviously we shouldn't use that with inout asm args.
Btw, I wonder if for strict-alignment targets asm()s can expect aligned
memory if they request an asm input with m? Thus, do we eventually
have to copy a known unaligned mem to aligned scratch memory before
passing it to a m input? Do we maybe have to do the same even for
m outputs? Or is this all simply undefined and asm()s have to handle
arbitrary alignment of memory operands (well, those that appear
at runtime, of course).
I'm sure the kernel people would rather *not* have copies to scratch
memory. The testcase in pr57586 was derived from kernel code that
munges pointers. A testcase that better shows what is going on,
probably from the same kernel code, is here
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57134#c3
(The pointer munging is what causes gcc to lose track of alignment.)
--
Alan Modra
Australia Development Lab, IBM
Re: expand_expr tweaks to fix PR57134
On Thu, Jun 13, 2013 at 10:45:38AM +0200, Richard Biener wrote:
On Wed, Jun 12, 2013 at 4:48 AM, Alan Modra amo...@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().
I suppose it also fixes PR57586 which looks similar?
Not completely. It cures the ICE, but you still get output number 0
not directly addressable. The reason being that expand_asm_operands
isn't asking for a mem. It should I guess, and also not specify
EXPAND_WRITE on an inout parameter. Bootstrapped and regression
tested powerpc64-linux.
PR middle-end/57586
* stmt.c (expand_asm_operands): Call expand_expr with
EXPAND_MEMORY for output operands that disallow regs. Don't
use EXPAND_WRITE on inout operands.
Index: gcc/stmt.c
===
--- gcc/stmt.c (revision 200083)
+++ gcc/stmt.c (working copy)
@@ -807,7 +807,10 @@ expand_asm_operands (tree string, tree outputs, tr
|| ! allows_reg
|| is_inout)
{
- op = expand_expr (val, NULL_RTX, VOIDmode, EXPAND_WRITE);
+ op = expand_expr (val, NULL_RTX, VOIDmode,
+ !allows_reg ? EXPAND_MEMORY
+ : !is_inout ? EXPAND_WRITE
+ : EXPAND_NORMAL);
if (MEM_P (op))
op = validize_mem (op);
Ok with a testcase and mentioning PR57586 in the changelog.
gcc.dg/compile/pr57134.c added.
--
Alan Modra
Australia Development Lab, IBM
Re: expand_expr tweaks to fix PR57134
On Fri, Jun 14, 2013 at 10:38 AM, Alan Modra amo...@gmail.com wrote:
On Thu, Jun 13, 2013 at 10:45:38AM +0200, Richard Biener wrote:
On Wed, Jun 12, 2013 at 4:48 AM, Alan Modra amo...@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().
I suppose it also fixes PR57586 which looks similar?
Not completely. It cures the ICE, but you still get output number 0
not directly addressable. The reason being that expand_asm_operands
isn't asking for a mem. It should I guess, and also not specify
EXPAND_WRITE on an inout parameter. Bootstrapped and regression
tested powerpc64-linux.
It looks reasonable to me, but I'm not too familiar with EXPAND_MEMORY
vs. EXPAND_WRITE.
Btw, I wonder if for strict-alignment targets asm()s can expect aligned
memory if they request an asm input with m? Thus, do we eventually
have to copy a known unaligned mem to aligned scratch memory before
passing it to a m input? Do we maybe have to do the same even for
m outputs? Or is this all simply undefined and asm()s have to handle
arbitrary alignment of memory operands (well, those that appear
at runtime, of course).
Richard.
PR middle-end/57586
* stmt.c (expand_asm_operands): Call expand_expr with
EXPAND_MEMORY for output operands that disallow regs. Don't
use EXPAND_WRITE on inout operands.
Index: gcc/stmt.c
===
--- gcc/stmt.c (revision 200083)
+++ gcc/stmt.c (working copy)
@@ -807,7 +807,10 @@ expand_asm_operands (tree string, tree outputs, tr
|| ! allows_reg
|| is_inout)
{
- op = expand_expr (val, NULL_RTX, VOIDmode, EXPAND_WRITE);
+ op = expand_expr (val, NULL_RTX, VOIDmode,
+ !allows_reg ? EXPAND_MEMORY
+ : !is_inout ? EXPAND_WRITE
+ : EXPAND_NORMAL);
if (MEM_P (op))
op = validize_mem (op);
Ok with a testcase and mentioning PR57586 in the changelog.
gcc.dg/compile/pr57134.c added.
--
Alan Modra
Australia Development Lab, IBM
Re: expand_expr tweaks to fix PR57134
On Wed, Jun 12, 2013 at 4:48 AM, Alan Modra amo...@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 kennerEXPAND_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 kennerMEMORY_USE_RO: the pointer reads memory.
14612 kennerMEMORY_USE_WO: the pointer writes to memory.
14612 kennerMEMORY_USE_RW: the pointer modifies memory (ie it reads
and writes). An
14612 kenner example is (*ptr)++
14612 kennerMEMORY_USE_BAD: use this if you don't know the behavior
of the pointer, or
14612 kennerif you know there are no pointers.
Using an INDIRECT_REF
14612 kennerwith MEMORY_USE_BAD will abort.
14612 kennerMEMORY_USE_TW: just test for writing, without update.
Special.
14612 kennerMEMORY_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
expand_expr tweaks to fix PR57134
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 kennerEXPAND_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 kennerMEMORY_USE_RO: the pointer reads memory.
14612 kennerMEMORY_USE_WO: the pointer writes to memory.
14612 kennerMEMORY_USE_RW: the pointer modifies memory (ie it reads
and writes). An
14612 kenner example is (*ptr)++
14612 kennerMEMORY_USE_BAD: use this if you don't know the behavior of
the pointer, or
14612 kennerif you know there are no pointers. Using
an INDIRECT_REF
14612 kennerwith MEMORY_USE_BAD will abort.
14612 kennerMEMORY_USE_TW: just test for writing, without update.
Special.
14612 kennerMEMORY_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?
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
-
