pgsql: Use static inline functions for float <-> Datum conversions.

Use static inline functions for float <-> Datum conversions.

Now that we are OK with using static inline functions, we can use them
to avoid function call overhead of pass-by-val versions of Float4GetDatum,
DatumGetFloat8, and Float8GetDatum. Those functions are only a few CPU
instructions long, but they could not be written into macros previously,
because we need a local union variable for the conversion.

I kept the pass-by-ref versions as regular functions. They are very simple
too, but they call palloc() anyway, so shaving a few instructions from the
function call doesn't seem so important there.

Discussion: <dbb82a4a-2c15-ba27-dd0a-009d2aa72b77(at)iki(dot)fi>

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/14cca1bf8e31ed39dbc26dd6c610f1113e759972

Modified Files
--------------
src/backend/utils/fmgr/fmgr.c | 63 +++++--------------------------------------
src/include/postgres.h | 63 +++++++++++++++++++++++++++++++++++++++++--
2 files changed, 67 insertions(+), 59 deletions(-)

Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi> writes:
> Use static inline functions for float <-> Datum conversions.

Hmm, it looks like narwhal for one is not happy with this:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=narwhal&dt=2016-08-31%2016%3A00%3A03

I suspect the failure traces to this bit in pg_enum.c:

/*
* On some machines, newelemorder may be in a register that's
* wider than float4. We need to force it to be rounded to float4
* precision before making the following comparisons, or we'll get
* wrong results. (Such behavior violates the C standard, but
* fixing the compilers is out of our reach.)
*/
newelemorder = DatumGetFloat4(Float4GetDatum(newelemorder));

If you suppose that inlining those macros allows gcc to decide that the
assignment is a no-op, the observed failure would be explained.

We had a bunch of similar problems in the recent work on exact degree trig
functions, and eventually found that storing values into volatile float8
variables was the most reliable way to force rounding to the expected
storage width. I propose to replace the above hack with declaring
newelemorder as volatile, and see if that makes things better.

regards, tom lane

On 08/31/2016 08:27 PM, Tom Lane wrote:
> Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi> writes:
>> Use static inline functions for float <-> Datum conversions.
>
> Hmm, it looks like narwhal for one is not happy with this:
> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=narwhal&dt=2016-08-31%2016%3A00%3A03
>
> I suspect the failure traces to this bit in pg_enum.c:
>
> /*
> * On some machines, newelemorder may be in a register that's
> * wider than float4. We need to force it to be rounded to float4
> * precision before making the following comparisons, or we'll get
> * wrong results. (Such behavior violates the C standard, but
> * fixing the compilers is out of our reach.)
> */
> newelemorder = DatumGetFloat4(Float4GetDatum(newelemorder));
>
> If you suppose that inlining those macros allows gcc to decide that the
> assignment is a no-op, the observed failure would be explained.

Ugh.

> We had a bunch of similar problems in the recent work on exact degree trig
> functions, and eventually found that storing values into volatile float8
> variables was the most reliable way to force rounding to the expected
> storage width. I propose to replace the above hack with declaring
> newelemorder as volatile, and see if that makes things better.

Sounds reasonable. None of this is supposed to be necessary, we're just
working around broken compilers, so whatever works.

I'll prepare a patch for that tomorrow, unless you're on it already.

- Heikki

Heikki Linnakangas <hlinnaka(at)iki(dot)fi> writes:
> On 08/31/2016 08:27 PM, Tom Lane wrote:
>> We had a bunch of similar problems in the recent work on exact degree trig
>> functions, and eventually found that storing values into volatile float8
>> variables was the most reliable way to force rounding to the expected
>> storage width. I propose to replace the above hack with declaring
>> newelemorder as volatile, and see if that makes things better.

> Sounds reasonable. None of this is supposed to be necessary, we're just
> working around broken compilers, so whatever works.

> I'll prepare a patch for that tomorrow, unless you're on it already.

On it already.

regards, tom lane