diff mbox

expand_expr tweaks to fix PR57134

Message ID 20130614083832.GK21523@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra June 14, 2013, 8:38 a.m. UTC
On Thu, Jun 13, 2013 at 10:45:38AM +0200, Richard Biener wrote:
> 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().
> 
> 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.


> Ok with a testcase and mentioning PR57586 in the changelog.

gcc.dg/compile/pr57134.c added.

Comments

Richard Biener June 14, 2013, 9:38 a.m. UTC | #1
On Fri, Jun 14, 2013 at 10:38 AM, Alan Modra <amodra@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 <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().
>>
>> 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
diff mbox

Patch

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);