Обсуждение: Defend against -ffast-math in meson builds
Hi hackers, while working on a patch (not shared yet), I had issues with floating-point and realized that we don't defend against -ffast-math in meson builds. We defend against in autoconf (because we don't want fast-math optimizations [1]), so the attached does the same for meson. [1]: https://postgr.es/m/424007.1644003689%40sss.pgh.pa.us Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Вложения
Hi, On Wed, 11 Mar 2026 at 14:52, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote: > > Hi hackers, > > while working on a patch (not shared yet), I had issues with floating-point and > realized that we don't defend against -ffast-math in meson builds. We defend > against in autoconf (because we don't want fast-math optimizations [1]), so the > attached does the same for meson. > > [1]: https://postgr.es/m/424007.1644003689%40sss.pgh.pa.us You are right. I applied the patch and confirmed that it works. -- Regards, Nazir Bilal Yavuz Microsoft
Hi, On 2026-03-11 11:52:28 +0000, Bertrand Drouvot wrote: > while working on a patch (not shared yet), I had issues with floating-point and > realized that we don't defend against -ffast-math in meson builds. We defend > against in autoconf (because we don't want fast-math optimizations [1]), so the > attached does the same for meson. I don't find this (nor the autoconf check) particularly likely to be helpful. The most likely way the flag would unintentionally be added would be via the cflags of some dependency - which won't be picked up by the tests. However we do have tests during the builds that should pick it up, in date.c and timestamp.c... Greetings, Andres Freund
On 11.03.26 14:43, Andres Freund wrote: > Hi, > > On 2026-03-11 11:52:28 +0000, Bertrand Drouvot wrote: >> while working on a patch (not shared yet), I had issues with floating-point and >> realized that we don't defend against -ffast-math in meson builds. We defend >> against in autoconf (because we don't want fast-math optimizations [1]), so the >> attached does the same for meson. > > I don't find this (nor the autoconf check) particularly likely to be > helpful. The most likely way the flag would unintentionally be added would be > via the cflags of some dependency - which won't be picked up by the tests. > > However we do have tests during the builds that should pick it up, in date.c > and timestamp.c... The existing check in configure is because certain Linux distributions used to compile everything with -ffast-math to be "faster", and that kept breaking PostgreSQL and so we wanted to stop them very early. These are gone, and the defenses in the code like date.c should be sufficient for any new attempts. I think we could remove the check in configure.
Peter Eisentraut <peter@eisentraut.org> writes:
> The existing check in configure is because certain Linux distributions
> used to compile everything with -ffast-math to be "faster", and that
> kept breaking PostgreSQL and so we wanted to stop them very early.
> These are gone, and the defenses in the code like date.c should be
> sufficient for any new attempts. I think we could remove the check in
> configure.
The defenses in those modules are probably obsolete too: aren't they about
ensuring exact results with floating-point timestamps? My gut reaction to
this was maybe we could remove *all* of that, so now I'm curious what
problem Bertrand ran into.
It could be that these old defenses are protecting us against roundoff
issues in some relatively-new floating-point functions, but if that's
true then we should move the checks to where they are important today.
regards, tom lane
Hi,
On Wed, Mar 11, 2026 at 03:07:10PM +0100, Peter Eisentraut wrote:
> On 11.03.26 14:43, Andres Freund wrote:
> > Hi,
> >
> > On 2026-03-11 11:52:28 +0000, Bertrand Drouvot wrote:
> > > while working on a patch (not shared yet), I had issues with floating-point and
> > > realized that we don't defend against -ffast-math in meson builds. We defend
> > > against in autoconf (because we don't want fast-math optimizations [1]), so the
> > > attached does the same for meson.
> >
> > I don't find this (nor the autoconf check) particularly likely to be
> > helpful. The most likely way the flag would unintentionally be added would be
> > via the cflags of some dependency - which won't be picked up by the tests.
> >
> > However we do have tests during the builds that should pick it up, in date.c
> > and timestamp.c...
>
> The existing check in configure is because certain Linux distributions used
> to compile everything with -ffast-math to be "faster", and that kept
> breaking PostgreSQL and so we wanted to stop them very early. These are
> gone, and the defenses in the code like date.c should be sufficient for any
> new attempts. I think we could remove the check in configure.
Oh, I did not see that we also have __FAST_MATH__ check in some C files. The reason
is that I build with:
CFLAGS="-ffast-math" CC="gcc" meson setup meson_build and -Dc_args="-Og -ggdb"
and that did not produce the error at compilation time.
But I got some regression tests failing like:
--- /home/postgres/postgresql/postgres/src/test/regress/expected/numeric.out
+++ /home/postgres/postgresql/postgres/meson_build/testrun/regress/regress/results/numeric.out
@@ -1630,22 +1630,7 @@
(0, 0, 1, 2147483647),
(1, 1, 0, 2147483647)
) as sample(oper, low, high, cnt);
- oper | low | high | cnt | width_bucket
--------------+-------------+-------------+------------+--------------
- 10.5 | -1.797e+308 | 1.797e+308 | 1 | 1
- 10.5 | -1.797e+308 | 1.797e+308 | 2 | 2
- 10.5 | -1.797e+308 | 1.797e+308 | 3 | 2
- 4.4925e+307 | -8.985e+307 | 8.985e+307 | 10 | 8
- 10.5 | 1.797e+308 | -1.797e+308 | 1 | 1
- 10.5 | 1.797e+308 | -1.797e+308 | 2 | 2
- 10.5 | 1.797e+308 | -1.797e+308 | 3 | 2
- 4.4925e+307 | 8.985e+307 | -8.985e+307 | 10 | 3
- 0 | 0 | 5e-324 | 4 | 1
- 5e-324 | 0 | 5e-324 | 4 | 5
- 0 | 0 | 1 | 2147483647 | 1
- 1 | 1 | 0 | 2147483647 | 1
-(12 rows)
-
+ERROR: "0.00000000..<LOT OF 0>..000005" is out of range for type double precision
or
--- /home/postgres/postgresql/postgres/src/test/regress/expected/money.out
+++ /home/postgres/postgresql/postgres/meson_build/testrun/regress/regress/results/money.out
@@ -536,7 +536,7 @@
SELECT '92233720368547758.07'::money * 2::float8;
ERROR: money out of range
SELECT '-1'::money / 1.175494e-38::float4;
-ERROR: money out of range
+ERROR: "0.00000000000000000000000000000000000001175494" is out of range for type real
But if I use:
CC="gcc" meson setup meson_build and -Dc_args="-Og -ggdb -ffast-math" then the
error happens at compilation time.
The check added in the patch would also catch the CFLAGS="-ffast-math" case.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hi, On Wed, Mar 11, 2026 at 11:19:45AM -0400, Tom Lane wrote: > Peter Eisentraut <peter@eisentraut.org> writes: > > The existing check in configure is because certain Linux distributions > > used to compile everything with -ffast-math to be "faster", and that > > kept breaking PostgreSQL and so we wanted to stop them very early. > > These are gone, and the defenses in the code like date.c should be > > sufficient for any new attempts. I think we could remove the check in > > configure. > > The defenses in those modules are probably obsolete too: aren't they about > ensuring exact results with floating-point timestamps? My gut reaction to > this was maybe we could remove *all* of that, so now I'm curious what > problem Bertrand ran into. I got some regression tests failing: [1]. [1]: https://postgr.es/m/abGO%2BBl1FQlpvFAt%40ip-10-97-1-34.eu-west-3.compute.internal Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> On Wed, Mar 11, 2026 at 11:19:45AM -0400, Tom Lane wrote:
>> The defenses in those modules are probably obsolete too: aren't they about
>> ensuring exact results with floating-point timestamps? My gut reaction to
>> this was maybe we could remove *all* of that, so now I'm curious what
>> problem Bertrand ran into.
> I got some regression tests failing: [1].
Yeah. I quickly tried it here (ie remove the defenses and build with
-ffast-math), and observed failures in a couple dozen core regression
tests. A bit of study suggests that
(1) both isnan() and isinf() tests are broken and always return false,
at least in some call sites;
(2) the code now seems exceedingly cavalier about near-overflow cases,
such as
regression=# select 1.175494e-38::float4;
ERROR: "0.00000000000000000000000000000000000001175494" is out of range for type real
which is a case that works fine normally.
So no, I don't wanna support this. But maybe we should move the
code-level tests out of the datetime files and into utils/float.h
or some such place.
regards, tom lane
Hi, On 2026-03-11 12:45:55 -0400, Tom Lane wrote: > So no, I don't wanna support this. But maybe we should move the > code-level tests out of the datetime files and into utils/float.h > or some such place. I think it's probably better to have it in a .c file (maybe float.c), I could kinda imagine some extension intentionally enabling -ffast-math, because it does something numerically intensive where the incorrectness doesn't matter. Greetings, Andres Freund
Hi, On Wed, Mar 11, 2026 at 12:45:55PM -0400, Tom Lane wrote: > Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes: > > On Wed, Mar 11, 2026 at 11:19:45AM -0400, Tom Lane wrote: > >> The defenses in those modules are probably obsolete too: aren't they about > >> ensuring exact results with floating-point timestamps? My gut reaction to > >> this was maybe we could remove *all* of that, so now I'm curious what > >> problem Bertrand ran into. > > > I got some regression tests failing: [1]. > > Yeah. I quickly tried it here (ie remove the defenses and build with > -ffast-math), and observed failures in a couple dozen core regression > tests. A bit of study suggests that > > (1) both isnan() and isinf() tests are broken and always return false, > at least in some call sites; > > (2) the code now seems exceedingly cavalier about near-overflow cases, > such as > > regression=# select 1.175494e-38::float4; > ERROR: "0.00000000000000000000000000000000000001175494" is out of range for type real > > which is a case that works fine normally. > > So no, I don't wanna support this. Yeah, that would be a nightmare to support. > But maybe we should move the > code-level tests out of the datetime files and into utils/float.h > or some such place. But still, I'm not sure the code-level guard is enough for meson. I think we need to put a guard in meson.build for the "oddity" described in [1]: it compiles fine with CFLAGS="-ffast-math" CC="gcc" meson setup meson_build but produces issues during the regression tests. I just had a closer look and it looks like that the reason is that it's being added at link time: $ cat meson_build/build.ninja | grep LINK_ARGS | grep -c "ffast-math" 254 So that the code-level is ok, but the tests fail. [1]: https://postgr.es/m/abGO%2BBl1FQlpvFAt%40ip-10-97-1-34.eu-west-3.compute.internal Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> But still, I'm not sure the code-level guard is enough for meson. I think we
> need to put a guard in meson.build for the "oddity" described in [1]:
> it compiles fine with CFLAGS="-ffast-math" CC="gcc" meson setup meson_build but
> produces issues during the regression tests.
> I just had a closer look and it looks like that the reason is that it's being
> added at link time:
> $ cat meson_build/build.ninja | grep LINK_ARGS | grep -c "ffast-math"
> 254
Oy. Surely that is a meson bug? AFAICS most of the effects of
-ffast-math should occur at compile time. Perhaps it also results
in linking a different version of libm, but that's not everything.
So just transposing it into link flags is not okay.
regards, tom lane
Hi,
On 2026-03-11 17:54:33 +0000, Bertrand Drouvot wrote:
> > But maybe we should move the
> > code-level tests out of the datetime files and into utils/float.h
> > or some such place.
>
> But still, I'm not sure the code-level guard is enough for meson. I think we
> need to put a guard in meson.build for the "oddity" described in [1]:
I think you rather need to get to the bottom of that issue.
> it compiles fine with CFLAGS="-ffast-math" CC="gcc" meson setup meson_build but
> produces issues during the regression tests.
This fails to build here as expected.
Are you sure that you actually specified it during meson setup and then didn't
reconfigure while the CFLAGs aren't specified?
> I just had a closer look and it looks like that the reason is that it's being
> added at link time:
>
> $ cat meson_build/build.ninja | grep LINK_ARGS | grep -c "ffast-math"
> 254
It's added to both, compile and link args, afaict?
andres@awork3:/tmp/pg-fast-math$ grep ffast-math build.ninja |awk '{print $1}'|sort|uniq -c
1679 ARGS
255 LINK_ARGS
Greetings,
Andres Freund
Hi, On Wed, Mar 11, 2026 at 02:03:28PM -0400, Tom Lane wrote: > Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes: > > But still, I'm not sure the code-level guard is enough for meson. I think we > > need to put a guard in meson.build for the "oddity" described in [1]: > > > it compiles fine with CFLAGS="-ffast-math" CC="gcc" meson setup meson_build but > > produces issues during the regression tests. > > > I just had a closer look and it looks like that the reason is that it's being > > added at link time: > > > $ cat meson_build/build.ninja | grep LINK_ARGS | grep -c "ffast-math" > > 254 > > Oy. Surely that is a meson bug? AFAICS most of the effects of > -ffast-math should occur at compile time. Perhaps it also results > in linking a different version of libm, but that's not everything. > So just transposing it into link flags is not okay. What I can see is that with CFLAGS="-ffast-math", I get: $ objdump -t meson_build/src/backend/postgres | grep crtfast 0000000000000000 l df *ABS* 0000000000000000 crtfastmath.c but it's empty without it. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Hi,
On Wed, Mar 11, 2026 at 02:10:54PM -0400, Andres Freund wrote:
> Hi,
>
> > it compiles fine with CFLAGS="-ffast-math" CC="gcc" meson setup meson_build but
> > produces issues during the regression tests.
>
> This fails to build here as expected.
>
> Are you sure that you actually specified it during meson setup and then didn't
> reconfigure while the CFLAGs aren't specified?
I was doing:
CFLAGS="-ffast-math" CC="gcc" meson setup meson_build
cd meson_build
meson configure -Db_coverage=true -Dbuildtype=debug -Dprefix=${PGINSTROOT} -Dpgport=${PGPORT} -Ddtrace=enabled
-Dc_args="-Og-frecord-gcc-switches"
ninja -v
ninja -v install
to produce the oddity.
>
> > I just had a closer look and it looks like that the reason is that it's being
> > added at link time:
> >
> > $ cat meson_build/build.ninja | grep LINK_ARGS | grep -c "ffast-math"
> > 254
>
> It's added to both, compile and link args, afaict?
>
> andres@awork3:/tmp/pg-fast-math$ grep ffast-math build.ninja |awk '{print $1}'|sort|uniq -c
> 1679 ARGS
> 255 LINK_ARGS
That's right (and fails at compilation time) if doing a one step approach:
CFLAGS="-ffast-math -Og -frecord-gcc-switches" CC="gcc" meson setup meson_build -Db_coverage=true -Dbuildtype=debug
-Dprefix=${PGINSTROOT}-Dpgport=${PGPORT} -Ddtrace=enabled
So, it's possible to produce this oddity with meson with the 2 steps approach,
while I don't think that would be possible with autoconf. That's why I think
the guard in meson.build would be a good thing to have, thoughts?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hi, On Wed, Mar 11, 2026 at 01:48:31PM -0400, Andres Freund wrote: > Hi, > > On 2026-03-11 12:45:55 -0400, Tom Lane wrote: > > So no, I don't wanna support this. But maybe we should move the > > code-level tests out of the datetime files and into utils/float.h > > or some such place. > > I think it's probably better to have it in a .c file (maybe float.c), I could > kinda imagine some extension intentionally enabling -ffast-math, because it > does something numerically intensive where the incorrectness doesn't matter. I think that you have a good point about the extension. That said a .h file could also prevent the extension to make use of -ffast-math "accidentally". And if they really want it, I think that they could compile the extension with "-ffast-math -U__FAST_MATH__" to make it work. I just tested it and that works but now that I write it that looks like an odd combination, so a .c file looks better. What do you think? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> On Wed, Mar 11, 2026 at 01:48:31PM -0400, Andres Freund wrote:
>> On 2026-03-11 12:45:55 -0400, Tom Lane wrote:
>>> So no, I don't wanna support this. But maybe we should move the
>>> code-level tests out of the datetime files and into utils/float.h
>>> or some such place.
>> I think it's probably better to have it in a .c file (maybe float.c), I could
>> kinda imagine some extension intentionally enabling -ffast-math, because it
>> does something numerically intensive where the incorrectness doesn't matter.
> I think that you have a good point about the extension. That said a .h file
> could also prevent the extension to make use of -ffast-math "accidentally".
Yeah, that was my thought too. But I think Andres has a point that in
principle an extension could use -ffast-math intentionally, so I'm
content to just put the test in float.c.
regards, tom lane
Hi, On Thu, Mar 12, 2026 at 11:10:51AM -0400, Tom Lane wrote: > Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes: > > On Wed, Mar 11, 2026 at 01:48:31PM -0400, Andres Freund wrote: > >> On 2026-03-11 12:45:55 -0400, Tom Lane wrote: > >>> So no, I don't wanna support this. But maybe we should move the > >>> code-level tests out of the datetime files and into utils/float.h > >>> or some such place. > > >> I think it's probably better to have it in a .c file (maybe float.c), I could > >> kinda imagine some extension intentionally enabling -ffast-math, because it > >> does something numerically intensive where the incorrectness doesn't matter. > > > I think that you have a good point about the extension. That said a .h file > > could also prevent the extension to make use of -ffast-math "accidentally". > > Yeah, that was my thought too. But I think Andres has a point that in > principle an extension could use -ffast-math intentionally, so I'm > content to just put the test in float.c. Yeah, I do agree. Something like in the attached? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Вложения
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> On Thu, Mar 12, 2026 at 11:10:51AM -0400, Tom Lane wrote:
>> Yeah, that was my thought too. But I think Andres has a point that in
>> principle an extension could use -ffast-math intentionally, so I'm
>> content to just put the test in float.c.
> Yeah, I do agree. Something like in the attached?
Pushed after fleshing out the comment in float.c a bit more.
regards, tom lane