diff mbox

expand_expr tweaks to fix PR57134

Message ID 20130913030720.GC22902@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra Sept. 13, 2013, 3:07 a.m. UTC 
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.


On Fri, Jun 14, 2013 at 11:38:58AM +0200, Richard Biener wrote:
> 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.

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

Comments

Alan Modra Sept. 24, 2013, 10:43 a.m. UTC | #1 
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?
Richard Biener Sept. 24, 2013, 10:46 a.m. UTC | #2 
On Tue, Sep 24, 2013 at 12:43 PM, Alan Modra <amodra@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
diff mbox

Patch

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