This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[libiberty] add obstack macros (was Re: PR #53525 - track-macro-expansion performance regression)


Hi Dodji, I appreciate your review but I'm replying for now only for the libiberty.h part (attached updated patch), which I've needed elsewhere and seems the easiest to quickly apply.

On Wed, 18 Jul 2012, Dodji Seketeli wrote:

=== modified file 'include/libiberty.h'
--- include/libiberty.h	2011-09-28 19:04:30 +0000
+++ include/libiberty.h	2012-07-07 16:04:01 +0000

[...]


-/* Type-safe obstack allocator.  */
+/* Type-safe obstack allocator. You must first initialize the obstack with
+   obstack_init() or _obstack_begin(). */

Why not just using the _obstack_begin function? Why introducing the use of the obstack_init macro?

Please correct me if I'm wrong, but my impression is that obstack_init
is the documented way (see [1]), _obstack_begin seems hidden and gives access to dangerous (performance-wise) parameters, even though we always use it with the defaults (hmmm double checking I see that we mostly set size 0 which is default, but also alignment 0???).


[1] http://www.gnu.org/software/libc/manual/html_node/Preparing-for-Obstacks.html


-#define XOBFINISH(O, T) ((T) obstack_finish ((O)))

I think this change is not needed. You basically remove this line to replace it with the hunk below, that comes later in the patch:

+#define XOBFINISH(O, PT) ((PT) obstack_finish ((O)))

So I believe you could do away with the change.



I just wanted to point out that it expects and returns a Pointer to the Type, e.g. after you XOBGROW(O, int, DATA) you should XOBFINISH(O, int *).
It would probably be better for XOBFINISH to accept T and return (T *), but I didn't want to change existing status quo.


I think I addressed your other concerns (style, comments, unneeded macro params). CC'd maintainers. What do you think about attached patch?


Thanks, Dimitris


P.S. Personally I prefer the original obstack interface, i.e. obstack_grow(), obstack_finish() etc. I just added the macros to follow existing style and avoid typecasting because of C++. Let me know if it's OK to use obstacks directly and I'll back out the patch.





2012-08-04 Dimitrios Apostolou <jimis@gmx.net>


	* libiberty.h
	(XOBDELETE,XOBGROW,XOBGROWVEC,XOBSHRINK,XOBSHRINKVEC): New
	type-safe macros for obstack allocation.
	(XOBFINISH): Renamed argument to PT since it is a pointer to T.

Attachment: libiberty.2.diff
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]