Patchwork [RFC] PR 55403 + 55391

login
register
mail settings
Submitter Richard Henderson
Date Nov. 20, 2012, 4:31 p.m.
Message ID <50ABB04E.5070705@redhat.com>
Download mbox | patch
Permalink /patch/200394/
State New
Headers show

Comments

Richard Henderson - Nov. 20, 2012, 4:31 p.m.
This assert looks to me like a "can this ever happen" sort of check.
It quite apparently can.

Although I'm a bit curious about the reasons we got to this point
in the 55403 instance (unaligned TCmode memory, extracting a TFmode
value), it's clear that one could intentionally write such a thing,
rather than have it happen by accident, and that it should work.

The following fills in what appears to be a blank.  I assume this
is the sort of thing you'd have intended?

I'm just doing another round of testing now...


r~
* emit-rtl.c (adjust_address_1): Handle adjust_object for size_known_p
	instead of asserting it isn't set.
Richard Sandiford - Nov. 20, 2012, 4:55 p.m.
Richard Henderson <rth@redhat.com> writes:
> This assert looks to me like a "can this ever happen" sort of check.
> It quite apparently can.
>
> Although I'm a bit curious about the reasons we got to this point
> in the 55403 instance (unaligned TCmode memory, extracting a TFmode
> value), it's clear that one could intentionally write such a thing,
> rather than have it happen by accident, and that it should work.
>
> The following fills in what appears to be a blank.  I assume this
> is the sort of thing you'd have intended?

Sorry, still haven't managed to reproduce this yet -- sparc bootstrap
still in slow progress, will try the alpha --with-long-double-128 thing
when I get home -- but what kind of bitfield memory were we trying to
create in the ICE case?  The idea was that "adjust_object" is only ever
true for bitfield adjustments.  We should then either be using an
integer or field mode whose size is picked up by:

  if (defattrs->size_known_p)
    size = defattrs->size;

or a BLKmode whose value is passed in via adjust_bitfield_address_size.
It sounds like I missed a case where the latter was needed.

Richard
Richard Henderson - Nov. 20, 2012, 5:10 p.m.
On 11/20/2012 08:55 AM, Richard Sandiford wrote:
> but what kind of bitfield memory were we trying to
> create in the ICE case?  The idea was that "adjust_object" is only ever
> true for bitfield adjustments.  We should then either be using an
> integer or field mode whose size is picked up by:
> 
>   if (defattrs->size_known_p)
>     size = defattrs->size;
> 
> or a BLKmode whose value is passed in via adjust_bitfield_address_size.
> It sounds like I missed a case where the latter was needed.

A TFmode field of an unaligned TCmode original.  We do wind up with
a BLKmode extraction, without the _size passed in.


r~

Patch

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 27464da..dd3339c 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -2177,7 +2177,11 @@  adjust_address_1 (rtx memref, enum machine_mode mode, HOST_WIDE_INT offset,
     }
   else if (attrs.size_known_p)
     {
-      gcc_assert (!adjust_object);
+      if (adjust_object)
+	{
+	  attrs.expr = NULL_TREE;
+	  attrs.alias = 0;
+	}
       attrs.size -= offset;
       /* ??? The store_by_pieces machinery generates negative sizes,
 	 so don't assert for that here.  */