Re: Unimpressed with pg_attribute_always_inline

On Tue, Jan 2, 2018 at 4:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>  baiji     | .\src\backend\executor\nodeHashjoin.c(165): warning C4141: 'inline' : used more than once
>  bowerbird | src/backend/executor/nodeHashjoin.c(165): warning C4141: 'inline' : used more than once
[c:\prog\bf\root\HEAD\pgsql.build\postgres.vcxproj]
>  currawong | .\src\backend\executor\nodeHashjoin.c(165): warning C4141: 'inline' : used more than once
>  gaur      | nodeHashjoin.c:167: warning: `always_inline' attribute directive ignored
>  mastodon  | .\src\backend\executor\nodeHashjoin.c(165): warning C4141: 'inline' : used more than once
>  thrips    | src/backend/executor/nodeHashjoin.c(165): warning C4141: 'inline' : used more than once
[C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj]
>  woodlouse | src/backend/executor/nodeHashjoin.c(165): warning C4141: 'inline' : used more than once
[C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj]

So that's two compilers:

1. MSVC doesn't like you to say both "__forceinline" and "inline".

2.  GCC 2.95.3 doesn't understand always_inline.  From a quick look at
archived manuals, it seems that that attribute arrived in 3.1.

It may be that "inline" can be removed (that seems to work OK for me
on clang, but I didn't check GCC).  Not sure off-hand how best to
tackle the ancient GCC problem; maybe a configure test or maybe a GCC
version test.  I will look into those problems.

> In the second place, what I read in gcc's manual about the meaning of
> the always_inline directive is
>
> `always_inline'
>      Generally, functions are not inlined unless optimization is
>      specified.  For functions declared inline, this attribute inlines
>      the function even if no optimization level was specified.
>
> I entirely reject the notion that we should be worried about optimizing
> performance in -O0 builds.  In fact, if someone is building with -O0,
> it's likely the case that they are hoping for exact correspondence
> of source lines to object code, and thus forcing inline is defeating
> their purpose.  I've certainly found plenty of times that inlining
> makes it harder to follow things in a debugger.
>
> Therefore, I think that pg_attribute_always_inline is not merely
> useless but actively bad, and should be removed.

My intention was to make sure it really did get inlined at higher
optimisation levels even though the compiler wouldn't otherwise choose
to do that in a couple of special cases, not to force inlining even at
-O0.  Not sure how to achieve the first of those things without the
second.  I wonder if there is a better way.

-- 
Thomas Munro
http://www.enterprisedb.com