Re: [PATCH] don't declare header-defined functions both static and inline

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Feb 16, 2023 at 08:37:34AM -0500, Patrick Palka wrote:
> I can confirm that this patch only modifies headers that reside in
> $prefix/lib/gcc/x86_64-pc-linux-gnu/13.0.1/plugin/include/
> (with --enable-languages=c,c++,fortran,objc,obj-c++ --enable-jit make install)
> 
> Good point, I was able to scrape the functions modified by this patch
> with the below shell script which outputs the name of each function that
> has at least two overloads/definitions (possibly in different headers),
> followed by the headers in which it's defined:
> 
> I manually verified that none of these function definitions conflict
> with each other.

> Bootstrapping with objc enabled revealed a minor manual changed was
> needed in gcc/objc/objc-act.cc to avoid a redeclaration mismatch error.
> 
> -- >8 --
> 
> Subject: [PATCH] don't declare header-defined functions both static and inline
> 
> Many functions defined in our headers are declared 'static inline' which
> is a C idiom that predates GCC's move to C++ as the implementation
> language.  But in C++ the inline keyword is more than just a compiler
> hint, and is sufficient to give the function the intended semantics.
> In fact declaring a function both static and inline is a pessimization
> since static effectively disables the desired definition merging
> behavior enabled by inline, and is also a source of (harmless) ODR
> violations when a static inline function gets called from a non-static
> inline one (such as tree_operand_check calling tree_operand_length).
> 
> This patch mechanically fixes the vast majority of occurrences of this
> anti-pattern throughout the compiler's headers via the command line
> 
>   echo gcc/*.h gcc/*/*.h | xargs sed -i 's/^static inline/inline/g'
> 
> The patch also manually removes the redundant declarations of is_ivar
> and lookup_category in gcc/objc/objc-act.cc which would otherwise
> conflict with those in objc-act.h (due to the difference in staticness).
> 
> Besides fixing some ODR violations, this speeds up stage1 cc1plus by
> about 2% and reduces the size of its text segment by 1.5MB.

Thanks for doing the extra work, LGTM for trunk then.

> gcc/ChangeLog:
> 
> 	* addresses.h: Mechanically drop 'static' from 'static inline'
> 	functions via s/^static inline/inline/g.
> 	* asan.h: Likewise.
> 	* attribs.h: Likewise.
> 	* basic-block.h: Likewise.
> 	* bitmap.h: Likewise.
> 	* cfghooks.h: Likewise.
> 	* cfgloop.h: Likewise.
> 	* cgraph.h: Likewise.
> 	* cselib.h: Likewise.
> 	* data-streamer.h: Likewise.
> 	* debug.h: Likewise.
> 	* df.h: Likewise.
> 	* diagnostic.h: Likewise.
> 	* dominance.h: Likewise.
> 	* dumpfile.h: Likewise.
> 	* emit-rtl.h: Likewise.
> 	* except.h: Likewise.
> 	* expmed.h: Likewise.
> 	* expr.h: Likewise.
> 	* fixed-value.h: Likewise.
> 	* gengtype.h: Likewise.
> 	* gimple-expr.h: Likewise.
> 	* gimple-iterator.h: Likewise.
> 	* gimple-predict.h: Likewise.
> 	* gimple-range-fold.h: Likewise.
> 	* gimple-ssa.h: Likewise.
> 	* gimple.h: Likewise.
> 	* graphite.h: Likewise.
> 	* hard-reg-set.h: Likewise.
> 	* hash-map.h: Likewise.
> 	* hash-set.h: Likewise.
> 	* hash-table.h: Likewise.
> 	* hwint.h: Likewise.
> 	* input.h: Likewise.
> 	* insn-addr.h: Likewise.
> 	* internal-fn.h: Likewise.
> 	* ipa-fnsummary.h: Likewise.
> 	* ipa-icf-gimple.h: Likewise.
> 	* ipa-inline.h: Likewise.
> 	* ipa-modref.h: Likewise.
> 	* ipa-prop.h: Likewise.
> 	* ira-int.h: Likewise.
> 	* ira.h: Likewise.
> 	* lra-int.h: Likewise.
> 	* lra.h: Likewise.
> 	* lto-streamer.h: Likewise.
> 	* memmodel.h: Likewise.
> 	* omp-general.h: Likewise.
> 	* optabs-query.h: Likewise.
> 	* optabs.h: Likewise.
> 	* plugin.h: Likewise.
> 	* pretty-print.h: Likewise.
> 	* range.h: Likewise.
> 	* read-md.h: Likewise.
> 	* recog.h: Likewise.
> 	* regs.h: Likewise.
> 	* rtl-iter.h: Likewise.
> 	* rtl.h: Likewise.
> 	* sbitmap.h: Likewise.
> 	* sched-int.h: Likewise.
> 	* sel-sched-ir.h: Likewise.
> 	* sese.h: Likewise.
> 	* sparseset.h: Likewise.
> 	* ssa-iterators.h: Likewise.
> 	* system.h: Likewise.
> 	* target-globals.h: Likewise.
> 	* target.h: Likewise.
> 	* timevar.h: Likewise.
> 	* tree-chrec.h: Likewise.
> 	* tree-data-ref.h: Likewise.
> 	* tree-iterator.h: Likewise.
> 	* tree-outof-ssa.h: Likewise.
> 	* tree-phinodes.h: Likewise.
> 	* tree-scalar-evolution.h: Likewise.
> 	* tree-sra.h: Likewise.
> 	* tree-ssa-alias.h: Likewise.
> 	* tree-ssa-live.h: Likewise.
> 	* tree-ssa-loop-manip.h: Likewise.
> 	* tree-ssa-loop.h: Likewise.
> 	* tree-ssa-operands.h: Likewise.
> 	* tree-ssa-propagate.h: Likewise.
> 	* tree-ssa-sccvn.h: Likewise.
> 	* tree-ssa.h: Likewise.
> 	* tree-ssanames.h: Likewise.
> 	* tree-streamer.h: Likewise.
> 	* tree-switch-conversion.h: Likewise.
> 	* tree-vectorizer.h: Likewise.
> 	* tree.h: Likewise.
> 	* wide-int.h: Likewise.
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-common.h: Mechanically drop static from static inline
> 	functions via s/^static inline/inline/g.
> 
> gcc/c/ChangeLog:
> 
> 	* c-parser.h: Mechanically drop static from static inline
> 	functions via s/^static inline/inline/g.
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h: Mechanically drop static from static inline
> 	functions via s/^static inline/inline/g.
> 
> gcc/fortran/ChangeLog:
> 
> 	* gfortran.h: Mechanically drop static from static inline
> 	functions via s/^static inline/inline/g.
> 
> gcc/jit/ChangeLog:
> 
> 	* jit-dejagnu.h: Mechanically drop static from static inline
> 	functions via s/^static inline/inline/g.
> 	* jit-recording.h: Likewise.
> 
> gcc/objc/ChangeLog:
> 
> 	* objc-act.h: Mechanically drop static from static inline
> 	functions via s/^static inline/inline/g.
> 	* objc-map.h: Likewise.
> 	* objc-act.cc: Remove the redundant redeclarations of is_ivar
> 	and lookup_category.

	Jakub