diff mbox

[1/7] Fix GTY markup of u2

Message ID 8761ld1k4o.fsf@talisman.default
State New
Headers show

Commit Message

Richard Sandiford May 10, 2014, 7:58 p.m. UTC
The rtx u2 field currently uses a desc/tag pair for GTY.  This seems
unnecessary though, since the field is specifically supposed to be
32 bits wide on 64-bit hosts and so cannot hold a pointer.

Tested on x86_64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
	* rtl.h (rtx_def): Mark u2 as GTY ((skip)).

Comments

Mike Stump May 11, 2014, 12:21 a.m. UTC | #1
On May 10, 2014, at 12:58 PM, Richard Sandiford <rdsandiford@googlemail.com> wrote:
> The rtx u2 field currently uses a desc/tag pair for GTY.  This seems
> unnecessary though,

> OK to install?

Ick.  I don’t favor skip.  The change feels like a premature optimization that doesn’t net any code gen benefit.  I’ll defer to a gty person if they prefer skip.
Michael Matz May 12, 2014, 10:19 a.m. UTC | #2
Hi,

On Sat, 10 May 2014, Mike Stump wrote:

> > The rtx u2 field currently uses a desc/tag pair for GTY.  This seems
> > unnecessary though,
> 
> > OK to install?
> 
> Ick.  I don’t favor skip.  The change feels like a premature 
> optimization that doesn’t net any code gen benefit.  I’ll defer to a gty 
> person if they prefer skip.

The skip is necessary, otherwise union members of GTY structs are required 
to have a 'desc' (and their members in turn are required to have a 'tag').  
So it's either the skip or the desc/tag pair.  The code-gen difference is 
one empty (but two-cased) switch statement less.


Ciao,
Michael.
Richard Sandiford May 12, 2014, 10:53 a.m. UTC | #3
Michael Matz <matz@suse.de> writes:
> Hi,
>
> On Sat, 10 May 2014, Mike Stump wrote:
>
>> > The rtx u2 field currently uses a desc/tag pair for GTY.  This seems
>> > unnecessary though,
>> 
>> > OK to install?
>> 
>> Ick.  I don’t favor skip.  The change feels like a premature 
>> optimization that doesn’t net any code gen benefit.  I’ll defer to a gty 
>> person if they prefer skip.
>
> The skip is necessary, otherwise union members of GTY structs are required 
> to have a 'desc' (and their members in turn are required to have a 'tag').  
> So it's either the skip or the desc/tag pair.  The code-gen difference is 
> one empty (but two-cased) switch statement less.

Yeah, but the reason I'm removing the desc/tag pair isn't so much to get
rid of that (the compiler should do it for us) but because marking it
anything other than "skip" gives the impression that we want to allow
GC pointers in the union.

Richard
Mike Stump May 12, 2014, 2:19 p.m. UTC | #4
On May 12, 2014, at 3:53 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote:
> Yeah, but the reason I'm removing the desc/tag pair isn't so much to get
> rid of that (the compiler should do it for us) but because marking it
> anything other than "skip" gives the impression that we want to allow
> GC pointers in the union.

Yeah, though I might like the generality of being able to do that, I will confess, I don’t think we will ever add a GC pointer, or indeed change the data much in the next 15 years.
Jeff Law May 12, 2014, 3:34 p.m. UTC | #5
On 05/10/14 13:58, Richard Sandiford wrote:
> The rtx u2 field currently uses a desc/tag pair for GTY.  This seems
> unnecessary though, since the field is specifically supposed to be
> 32 bits wide on 64-bit hosts and so cannot hold a pointer.
>
> Tested on x86_64-linux-gnu.  OK to install?
>
> Thanks,
> Richard
>
>
> gcc/
> 	* rtl.h (rtx_def): Mark u2 as GTY ((skip)).
OK for the trunk.

Jeff
diff mbox

Patch

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2014-05-10 09:35:05.112643742 +0100
+++ gcc/rtl.h	2014-05-10 14:02:52.349291772 +0100
@@ -354,8 +354,8 @@  struct GTY((chain_next ("RTX_NEXT (&%h)"
 
     /* In a CONST_WIDE_INT (aka hwivec_def), this is the number of
        HOST_WIDE_INTs in the hwivec_def.  */
-    unsigned GTY ((tag ("CONST_WIDE_INT"))) num_elem:32;
-  } GTY ((desc ("GET_CODE (&%0)"))) u2;
+    unsigned num_elem;
+  } GTY ((skip)) u2;
 
   /* The first element of the operands of this rtx.
      The number of operands and their types are controlled