Thread: Re: [COMMITTERS] pgsql: Use static inline functions for float <-> Datum conversions.

Heikki Linnakangas <heikki.linnakangas@iki.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.
Weneed 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.linnakangas@iki.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@iki.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