[PATCH] cleanup gcse.c:canon_modify_mem_list

[PATCH] cleanup gcse.c:canon_modify_mem_list

[Nathan Froyd]

The patch below converts gcse.c:canon_modify_mem_list to hold VECs
instead of EXPR_LIST rtxes.  I am ambivalent about the use of VECs in
canon_modify_mem_list; they will waste some memory compared to the
linked list scheme present before, though I'm not sure how much.  It
would depend on the average chain length, etc.  I'm happy to use an
linked list datastructure instead, allocated out of an
alloc_pool (better statistics!) or even gcse's private obstack if folks
think that would be better.  Moving things out of GC memory and
eliminating a use of EXPR_LIST has to be considered a good thing,
though...

Doing this required addressing an odd little comment in
record_last_mem_set_info:

  if (CALL_P (insn))
    {
      /* Note that traversals of this loop (other than for free-ing)
	 will break after encountering a CALL_INSN.  So, there's no
	 need to insert a pair of items, as canon_list_insert does.  */
      canon_modify_mem_list[bb] =
	alloc_INSN_LIST (insn, canon_modify_mem_list[bb]);
      bitmap_set_bit (blocks_with_calls, bb);
    }

This is all well and good, except that the only real traversal of
canon_modify_mem_list (compute_transp) doesn't check for CALL_INSNs.
Instead, it has:

	    EXECUTE_IF_AND_COMPL_IN_BITMAP (modify_mem_list_set,
					    blocks_with_calls,
					    0, bb_index, bi)
	      {
		rtx list_entry = canon_modify_mem_list[bb_index];

		while (list_entry)
		  {
		    rtx dest, dest_addr;

		    /* LIST_ENTRY must be an INSN of some kind that sets memory.
		       Examine each hunk of memory that is modified.  */

		    dest = XEXP (list_entry, 0);
		    list_entry = XEXP (list_entry, 1);
		    dest_addr = XEXP (list_entry, 0);

Notice that since bits in blocks_with_calls are set if we find a
CALL_INSN, we'll never examine canon_modify_mem_list for such blocks.
Hence we can dispense with the spurious consing in
record_last_mem_set_info.

Tested on x86_64-unknown-linux-gnu.  OK to commit?

-Nathan

	* gcse.c (modify_pair): Define.  Define a VEC of it.
	(canon_modify_mem_list): Convert to an array of VECs.
	(free_insn_expr_list_list): Delete.
	(clear_modify_mem_tables): Call VEC_free instead.
	(record_last_mem_set_info): Don't modify canon_modify_mem_list.
	(alloc_gcse_mem): Adjust for canon_modify_mem_list change.
	(canon_list_insert, compute_transp): Likewise.
---
 gcc/gcse.c |   79 +++++++++++++++++++++--------------------------------------
 1 files changed, 28 insertions(+), 51 deletions(-)

diff --git a/gcc/gcse.c b/gcc/gcse.c
index a1de61f..d6a4db4 100644
--- a/gcc/gcse.c
+++ b/gcc/gcse.c
@@ -385,8 +385,18 @@ static regset reg_set_bitmap;
 static rtx * modify_mem_list;
 static bitmap modify_mem_list_set;
 
-/* This array parallels modify_mem_list, but is kept canonicalized.  */
-static rtx * canon_modify_mem_list;
+typedef struct modify_pair_s
+{
+  rtx dest;			/* A MEM.  */
+  rtx dest_addr;		/* The canonical address of `dest'.  */
+} modify_pair;
+
+DEF_VEC_O(modify_pair);
+DEF_VEC_ALLOC_O(modify_pair,heap);
+
+/* This array parallels modify_mem_list, except that it stores MEMs
+   being set and their canonicalized memory addresses.  */
+static VEC(modify_pair,heap) **canon_modify_mem_list;
 
 /* Bitmap indexed by block numbers to record which blocks contain
    function calls.  */
@@ -478,7 +488,6 @@ static void invalidate_any_buried_refs (rtx);
 static void compute_ld_motion_mems (void);
 static void trim_ld_motion_mems (void);
 static void update_ld_motion_stores (struct expr *);
-static void free_insn_expr_list_list (rtx *);
 static void clear_modify_mem_tables (void);
 static void free_modify_mem_tables (void);
 static rtx gcse_emit_move_after (rtx, rtx, rtx);
@@ -587,7 +596,8 @@ alloc_gcse_mem (void)
   /* Allocate array to keep a list of insns which modify memory in each
      basic block.  */
   modify_mem_list = GCNEWVEC (rtx, last_basic_block);
-  canon_modify_mem_list = GCNEWVEC (rtx, last_basic_block);
+  canon_modify_mem_list = GCNEWVEC (VEC(modify_pair,heap) *,
+				    last_basic_block);
   modify_mem_list_set = BITMAP_ALLOC (NULL);
   blocks_with_calls = BITMAP_ALLOC (NULL);
 }
@@ -1435,6 +1445,7 @@ canon_list_insert (rtx dest ATTRIBUTE_UNUSED, const_rtx unused1 ATTRIBUTE_UNUSED
 {
   rtx dest_addr, insn;
   int bb;
+  modify_pair *pair;
 
   while (GET_CODE (dest) == SUBREG
       || GET_CODE (dest) == ZERO_EXTRACT
@@ -1453,10 +1464,9 @@ canon_list_insert (rtx dest ATTRIBUTE_UNUSED, const_rtx unused1 ATTRIBUTE_UNUSED
   insn = (rtx) v_insn;
   bb = BLOCK_FOR_INSN (insn)->index;
 
-  canon_modify_mem_list[bb] =
-    alloc_EXPR_LIST (VOIDmode, dest_addr, canon_modify_mem_list[bb]);
-  canon_modify_mem_list[bb] =
-    alloc_EXPR_LIST (VOIDmode, dest, canon_modify_mem_list[bb]);
+  pair = VEC_safe_push (modify_pair, heap, canon_modify_mem_list[bb], NULL);
+  pair->dest = dest;
+  pair->dest_addr = dest_addr;
 }
 
 /* Record memory modification information for INSN.  We do not actually care
@@ -1474,14 +1484,7 @@ record_last_mem_set_info (rtx insn)
   bitmap_set_bit (modify_mem_list_set, bb);
 
   if (CALL_P (insn))
-    {
-      /* Note that traversals of this loop (other than for free-ing)
-	 will break after encountering a CALL_INSN.  So, there's no
-	 need to insert a pair of items, as canon_list_insert does.  */
-      canon_modify_mem_list[bb] =
-	alloc_INSN_LIST (insn, canon_modify_mem_list[bb]);
-      bitmap_set_bit (blocks_with_calls, bb);
-    }
+    bitmap_set_bit (blocks_with_calls, bb);
   else
     note_stores (PATTERN (insn), canon_list_insert, (void*) insn);
 }
@@ -1609,26 +1612,6 @@ compute_hash_table (struct hash_table_d *table)
 \f
 /* Expression tracking support.  */
 
-/* Like free_INSN_LIST_list or free_EXPR_LIST_list, except that the node
-   types may be mixed.  */
-
-static void
-free_insn_expr_list_list (rtx *listp)
-{
-  rtx list, next;
-
-  for (list = *listp; list ; list = next)
-    {
-      next = XEXP (list, 1);
-      if (GET_CODE (list) == EXPR_LIST)
-	free_EXPR_LIST_node (list);
-      else
-	free_INSN_LIST_node (list);
-    }
-
-  *listp = NULL;
-}
-
 /* Clear canon_modify_mem_list and modify_mem_list tables.  */
 static void
 clear_modify_mem_tables (void)
@@ -1639,7 +1622,7 @@ clear_modify_mem_tables (void)
   EXECUTE_IF_SET_IN_BITMAP (modify_mem_list_set, 0, i, bi)
     {
       free_INSN_LIST_list (modify_mem_list + i);
-      free_insn_expr_list_list (canon_modify_mem_list + i);
+      VEC_free (modify_pair, heap, canon_modify_mem_list[i]);
     }
   bitmap_clear (modify_mem_list_set);
   bitmap_clear (blocks_with_calls);
@@ -1710,25 +1693,19 @@ compute_transp (const_rtx x, int indx, sbitmap *bmap)
 					    blocks_with_calls,
 					    0, bb_index, bi)
 	      {
-		rtx list_entry = canon_modify_mem_list[bb_index];
+		VEC(modify_pair,heap) *list
+		  = canon_modify_mem_list[bb_index];
+		modify_pair *pair;
+		unsigned ix;
 
-		while (list_entry)
+		FOR_EACH_VEC_ELT_REVERSE (modify_pair, list, ix, pair)
 		  {
-		    rtx dest, dest_addr;
-
-		    /* LIST_ENTRY must be an INSN of some kind that sets memory.
-		       Examine each hunk of memory that is modified.  */
-
-		    dest = XEXP (list_entry, 0);
-		    list_entry = XEXP (list_entry, 1);
-		    dest_addr = XEXP (list_entry, 0);
+		    rtx dest = pair->dest;
+		    rtx dest_addr = pair->dest_addr;
 
 		    if (canon_true_dependence (dest, GET_MODE (dest), dest_addr,
 					       x, NULL_RTX, rtx_addr_varies_p))
-		      {
-			RESET_BIT (bmap[bb_index], indx);
-		      }
-		    list_entry = XEXP (list_entry, 1);
+		      RESET_BIT (bmap[bb_index], indx);
 	          }
 	      }
 	}
-- 
1.7.0.4

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Jeff Law]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/03/11 19:44, Nathan Froyd wrote:
> The patch below converts gcse.c:canon_modify_mem_list to hold VECs
> instead of EXPR_LIST rtxes.  I am ambivalent about the use of VECs in
> canon_modify_mem_list; they will waste some memory compared to the
> linked list scheme present before, though I'm not sure how much.  It
> would depend on the average chain length, etc.  I'm happy to use an
> linked list datastructure instead, allocated out of an
> alloc_pool (better statistics!) or even gcse's private obstack if folks
> think that would be better.  Moving things out of GC memory and
> eliminating a use of EXPR_LIST has to be considered a good thing,
> though...
I've got no strong opinions on all this stuff -- except that blindly
moving stuff out of GC memory isn't necessarily a good thing.  It really
depends on the lifetime of the objects.

Assuming the memory list stuff in gcse doesn't have a lifetime outside
of GCSE and is thus easily tracked, then I've got no fundamental
objection to the change.


> 
> Doing this required addressing an odd little comment in
> record_last_mem_set_info:
> 
>   if (CALL_P (insn))
>     {
>       /* Note that traversals of this loop (other than for free-ing)
> 	 will break after encountering a CALL_INSN.  So, there's no
> 	 need to insert a pair of items, as canon_list_insert does.  */
>       canon_modify_mem_list[bb] =
> 	alloc_INSN_LIST (insn, canon_modify_mem_list[bb]);
>       bitmap_set_bit (blocks_with_calls, bb);
>     }
> 
> This is all well and good, except that the only real traversal of
> canon_modify_mem_list (compute_transp) doesn't check for CALL_INSNs.
It's possible (likely?) the implementation changed over time and the
comment wasn't properly updated.  Unfortunate, but it does happen.


> +  canon_modify_mem_list = GCNEWVEC (VEC(modify_pair,heap) *,
> +				    last_basic_block);
nit; You're missing some whitespace here (after the VEC).

OK.  Please install,

Thanks,
Jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNmgdkAAoJEBRtltQi2kC7QagH/AkchggJ4C7SU2AasolDyQqn
tcQowd5zBmiYFujY9+UgIL6Wh6AVU/Ls452c96MVKKWcDi8kIW0y3tzlls5yYbKW
/XtvuzPU9zhya672mjTNktD3mPFj4qKtAO7PsjCh375uvkSknSXCAP9B5O9nQPbR
BdaaAHv4gLgrpIokFTxk5455/7BGMCNJ0/O91PR4Jyithc2wZsz6Me4AFg+aMZG/
t+Vq7+6D5kALiXrrn2UNzrGefE6i6HdbacP6drOaDI1XNmI8Se4NgiE/JQfkvKty
1i4MVGW2IJrMax7fCKLhIRErQxEgfGQVfOLk5WkQXSzxfvILLu1bkdTTLKTr6t0=
=tTk7
-----END PGP SIGNATURE-----

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Nathan Froyd]

On Mon, Apr 04, 2011 at 12:01:09PM -0600, Jeff Law wrote:
> > +  canon_modify_mem_list = GCNEWVEC (VEC(modify_pair,heap) *,
> > +				    last_basic_block);
> nit; You're missing some whitespace here (after the VEC).

This doesn't seem to be a hard-and-fast policy; all of the VEC code I
remember writing or looking at recently has no spaces, and I checked the
patch in on that basis.  However, running grep indicates that we have a
profusion of styles...

-Nathan

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Richard Earnshaw]

On Tue, 2011-04-05 at 04:44 -0700, Nathan Froyd wrote:
> On Mon, Apr 04, 2011 at 12:01:09PM -0600, Jeff Law wrote:
> > > +  canon_modify_mem_list = GCNEWVEC (VEC(modify_pair,heap) *,
> > > +				    last_basic_block);
> > nit; You're missing some whitespace here (after the VEC).
> 
> This doesn't seem to be a hard-and-fast policy; all of the VEC code I
> remember writing or looking at recently has no spaces, and I checked the
> patch in on that basis.  However, running grep indicates that we have a
> profusion of styles...

I think the style guide is quite clear on this: there should be a space
there as Jeff says.  The fact that some code has crept in with other
styles doesn't make them right, or give justification for ignoring the
style guide.

R.

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Nathan Froyd]

On Tue, Apr 05, 2011 at 01:22:39PM +0100, Richard Earnshaw wrote:
> On Tue, 2011-04-05 at 04:44 -0700, Nathan Froyd wrote:
> > > nit; You're missing some whitespace here (after the VEC).
> > 
> > This doesn't seem to be a hard-and-fast policy; all of the VEC code I
> > remember writing or looking at recently has no spaces, and I checked the
> > patch in on that basis.  However, running grep indicates that we have a
> > profusion of styles...
> 
> I think the style guide is quite clear on this: there should be a space
> there as Jeff says.  The fact that some code has crept in with other
> styles doesn't make them right, or give justification for ignoring the
> style guide.

Would you like a patch for the hundreds of instances without spaces?

Certainly vec.h never uses spaces; I thought this was simply The Way
Things Were.

-Nathan

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Richard Earnshaw]

On Tue, 2011-04-05 at 05:30 -0700, Nathan Froyd wrote:
> On Tue, Apr 05, 2011 at 01:22:39PM +0100, Richard Earnshaw wrote:
> > On Tue, 2011-04-05 at 04:44 -0700, Nathan Froyd wrote:
> > > > nit; You're missing some whitespace here (after the VEC).
> > > 
> > > This doesn't seem to be a hard-and-fast policy; all of the VEC code I
> > > remember writing or looking at recently has no spaces, and I checked the
> > > patch in on that basis.  However, running grep indicates that we have a
> > > profusion of styles...
> > 
> > I think the style guide is quite clear on this: there should be a space
> > there as Jeff says.  The fact that some code has crept in with other
> > styles doesn't make them right, or give justification for ignoring the
> > style guide.
> 
> Would you like a patch for the hundreds of instances without spaces?
> 
> Certainly vec.h never uses spaces; I thought this was simply The Way
> Things Were.
> 
> -Nathan
> 

IMO, rototils are generally pointless.  If you are fixing code nearby
then yes, fix the formatting issues.  Otherwise, just don't exacerbate
the problem, or we'll reach the point where a rototil really does become
necessary.

R.

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Jeff Law]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/05/11 06:55, Richard Earnshaw wrote:
> 
> On Tue, 2011-04-05 at 05:30 -0700, Nathan Froyd wrote:
>> On Tue, Apr 05, 2011 at 01:22:39PM +0100, Richard Earnshaw wrote:
>>> On Tue, 2011-04-05 at 04:44 -0700, Nathan Froyd wrote:
>>>>> nit; You're missing some whitespace here (after the VEC).
>>>>
>>>> This doesn't seem to be a hard-and-fast policy; all of the VEC code I
>>>> remember writing or looking at recently has no spaces, and I checked the
>>>> patch in on that basis.  However, running grep indicates that we have a
>>>> profusion of styles...
>>>
>>> I think the style guide is quite clear on this: there should be a space
>>> there as Jeff says.  The fact that some code has crept in with other
>>> styles doesn't make them right, or give justification for ignoring the
>>> style guide.
>>
>> Would you like a patch for the hundreds of instances without spaces?
>>
>> Certainly vec.h never uses spaces; I thought this was simply The Way
>> Things Were.
>>
>> -Nathan
>>
> 
> IMO, rototils are generally pointless.  If you are fixing code nearby
> then yes, fix the formatting issues.  Otherwise, just don't exacerbate
> the problem, or we'll reach the point where a rototil really does become
> necessary.
Well, the other approach is to have a commit hook which automatically
deals with formatting crap by running indent, or whatever tool we want.

That would take the decision out of the hands of the developer and free
the reviewer from having to catch such things and ensures there is a
canonical form for our sources.

jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNmxQQAAoJEBRtltQi2kC7f7sH/RtTsEdYZKbgD1IdDTHnAW2R
OW3ie04GuUdFC1ZAekvOcVhbIeouLZ/HyyaWiZZ3uNajF6cRDMIDe7MEygqKAWKg
WUVtJeQlrhnERcvjJFe3pYzxf2wBwSYI/A+6Ql5mXfigx2Za7WoSdzq1zQXvd+Pe
ihR35DClH2lX3YAqGi6h47J+lk0kRuN1kvnSWwOzo/ACYBkdJrbZDCRVtkV+UQnt
zgn/NkUwNbnJmyApLlr5jVYRIbe8saVrjnrO39siOVz26eqnuV8IehY7ePnidr3f
8wZ06mTJaN/PgMpCltHvQZ3Vos/+BxTtCMTAFaxRKAJd25HNcd955GHBNG7qIYE=
=UkkH
-----END PGP SIGNATURE-----

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Richard Earnshaw]

On Tue, 2011-04-05 at 07:07 -0600, Jeff Law wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 04/05/11 06:55, Richard Earnshaw wrote:
> > 
> > On Tue, 2011-04-05 at 05:30 -0700, Nathan Froyd wrote:
> >> On Tue, Apr 05, 2011 at 01:22:39PM +0100, Richard Earnshaw wrote:
> >>> On Tue, 2011-04-05 at 04:44 -0700, Nathan Froyd wrote:
> >>>>> nit; You're missing some whitespace here (after the VEC).
> >>>>
> >>>> This doesn't seem to be a hard-and-fast policy; all of the VEC code I
> >>>> remember writing or looking at recently has no spaces, and I checked the
> >>>> patch in on that basis.  However, running grep indicates that we have a
> >>>> profusion of styles...
> >>>
> >>> I think the style guide is quite clear on this: there should be a space
> >>> there as Jeff says.  The fact that some code has crept in with other
> >>> styles doesn't make them right, or give justification for ignoring the
> >>> style guide.
> >>
> >> Would you like a patch for the hundreds of instances without spaces?
> >>
> >> Certainly vec.h never uses spaces; I thought this was simply The Way
> >> Things Were.
> >>
> >> -Nathan
> >>
> > 
> > IMO, rototils are generally pointless.  If you are fixing code nearby
> > then yes, fix the formatting issues.  Otherwise, just don't exacerbate
> > the problem, or we'll reach the point where a rototil really does become
> > necessary.
> Well, the other approach is to have a commit hook which automatically
> deals with formatting crap by running indent, or whatever tool we want.
> 
> That would take the decision out of the hands of the developer and free
> the reviewer from having to catch such things and ensures there is a
> canonical form for our sources.

Only if a tool either

1) Has a way to over-ride it when it's being stupid, or...
2) is never stupid...

Most indent tools don't handle comments very well, IMO.  For example:

  /* Some commentary text...

	Some Code Fragment

     some more commentary text.  */

if you auto-indent that, you'll often end up with the code fragment,
which was deliberately given extra indentation being moved to the same
indentation as the surrounding text.  In some cases that can lose
important information.

R.

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Jeff Law]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/05/11 05:44, Nathan Froyd wrote:
> On Mon, Apr 04, 2011 at 12:01:09PM -0600, Jeff Law wrote:
>>> +  canon_modify_mem_list = GCNEWVEC (VEC(modify_pair,heap) *,
>>> +				    last_basic_block);
>> nit; You're missing some whitespace here (after the VEC).
> 
> This doesn't seem to be a hard-and-fast policy; all of the VEC code I
> remember writing or looking at recently has no spaces, and I checked the
> patch in on that basis.  However, running grep indicates that we have a
> profusion of styles...
In a codebase as big as GCC, violations tends to slip in.    If/when you
see stuff, we'd love to see patches which fix such problems.

jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNm0EOAAoJEBRtltQi2kC72/gH+gO3bpEldTzuEblKU5XGyhRw
h92X43nPb7qZ/9SaH9RI+zc0XGRAclwqja7X360kFmC/b06AMTbakmsBkcIZDFUj
MhXnt5jsOtQGAA4UlA1rPaUTA8IPN+QzIfIe8puN8Yf/W5Q0nxxKzJPbujXLkK0o
+5dB8CXUOYQ0rAfkzleEUSdPi0iKm8wvg13HY83Q52EZvUQ3aWcxFPmXA0oIaz+v
FQVhjjzr4YvHd2MaomofWC/t246bLVB+L6ofZnjh0jkhAikC0/z+FLG8en0VfGhr
0it6/hRIchlZfRV6jh/dN6vLR+NwP8FHwG25O/G+jnPRbut3fw3BaaQr4n3dnTQ=
=vAx7
-----END PGP SIGNATURE-----

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Mike Stump]

On Apr 5, 2011, at 5:55 AM, Richard Earnshaw wrote:
> IMO, rototils are generally pointless.

As a counter point, I like polish and style, in addition to beauty and flexibility.  I'd rather see more style cleanups, more polish cleanups and more beautiful cleanups.  I'd like to see more, not less people doing that work.  I'd like to see more maintainership status give out on that basis.  I like rototils.  This is the mechanism by which we give a uniformity to the code, make it predictable, easier to work, easier to copy from.

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Joseph S. Myers]

On Tue, 5 Apr 2011, Nathan Froyd wrote:

> On Tue, Apr 05, 2011 at 01:22:39PM +0100, Richard Earnshaw wrote:
> > On Tue, 2011-04-05 at 04:44 -0700, Nathan Froyd wrote:
> > > > nit; You're missing some whitespace here (after the VEC).
> > > 
> > > This doesn't seem to be a hard-and-fast policy; all of the VEC code I
> > > remember writing or looking at recently has no spaces, and I checked the
> > > patch in on that basis.  However, running grep indicates that we have a
> > > profusion of styles...
> > 
> > I think the style guide is quite clear on this: there should be a space
> > there as Jeff says.  The fact that some code has crept in with other
> > styles doesn't make them right, or give justification for ignoring the
> > style guide.
> 
> Would you like a patch for the hundreds of instances without spaces?
> 
> Certainly vec.h never uses spaces; I thought this was simply The Way
> Things Were.

I also had the impression that for certain special macros such as VEC, 
GTY, _, N_, G_ - macros that are not really substituting for functions, 
for-loops, etc. - the norm was no space, whereas for function prototypes, 
function calls and calls to macros that are being used syntactically and 
semantically more or less like functions the norm is to have the space 
(I'm not sure about the case of __attribute__ and macros generating 
__attribute__).  But I see VEC is in fact used more often with the space.  
(For the VEC_* macros used like functions rather than in type names, the 
norm is very clearly to have the space.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Nathan Froyd]

On Tue, Apr 05, 2011 at 07:18:16PM +0000, Joseph S. Myers wrote:
> On Tue, 5 Apr 2011, Nathan Froyd wrote:
> > Certainly vec.h never uses spaces; I thought this was simply The Way
> > Things Were.
> 
> I also had the impression that for certain special macros such as VEC, 
> GTY, _, N_, G_ - macros that are not really substituting for functions, 
> for-loops, etc. - the norm was no space, whereas for function prototypes, 
> function calls and calls to macros that are being used syntactically and 
> semantically more or less like functions the norm is to have the space 
> (I'm not sure about the case of __attribute__ and macros generating 
> __attribute__).  But I see VEC is in fact used more often with the space.  

To be pedantic: grepping for 'VEC (' in gcc/ gives ~1750 hits.  But a
number of those are the X*VEC allocation functions and things like
CLASSTYPE_METHOD_VEC; cutting those out ("fgrep 'VEC (' | egrep -v
'[A-Z][A-Z_]+VEC ('") or similar gives a bit over 800 VEC instances that
use spaces.

grepping for 'VEC(' gives 1300+ hits.  That's a lot of creeping laxity. :)

-Nathan

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Nathan Froyd]

On Mon, Apr 04, 2011 at 05:42:42PM +0200, Steven Bosscher wrote:
> Nice cleanup. I can't approve it but it looks alright to me. I suppose
> you're planning something similar for modify_mem_list?

That's the plan, along with numerous other users of {INSN,EXPR}_LIST.

-Nathan

Re: [PATCH] cleanup gcse.c:canon_modify_mem_list

[Steven Bosscher]

Hi Nathan,

Nice cleanup. I can't approve it but it looks alright to me. I suppose
you're planning something similar for modify_mem_list?

Ciao!
Steven