libexpr: Switch parser.y to %skeleton lalr1.cc #14105
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since the parser is now LALR we can easily switch over to the less ugly sketelon than the default C one. This would allow us to switch from %union to %define api.value.type variant in the future to avoid the need for triviall POD types.
xokdvium
force-pushed
the
cpp-bison
branch
2 times, most recently
from
f6cee0d to
45a5665
Compare
xokdvium
commented
Sep 28, 2025
Comment on lines
167
to
184
| preConfigure = | ||
| let | ||
| interpositionFlags = [ | ||
| "-fno-semantic-interposition" | ||
| "-Wl,-Bsymbolic-functions" | ||
| ]; | ||
| in | ||
| # NOTE: By default GCC disables interprocedular optimizations (in particular inlining) for | ||
| # position-independent code and thus shared libraries. | ||
| # Since LD_PRELOAD tricks aren't worth losing out on optimizations, we disable it for good. | ||
| # This is not the case for Clang, where -fno-semantic-interposition is the default. | ||
| # https://reviews.llvm.org/D102453 | ||
| # https://fedoraproject.org/wiki/Changes/PythonNoSemanticInterpositionSpeedup | ||
| prevAttrs.preConfigure or "" | ||
| + lib.optionalString stdenv.cc.isGNU '' | ||
| export CFLAGS="''${CFLAGS:-} ${toString interpositionFlags}" | ||
| export CXXFLAGS="''${CXXFLAGS:-} ${toString interpositionFlags}" | ||
| ''; |
There was a problem hiding this comment.
We should remember to bring this to nixpkgs
There was a problem hiding this comment.
we can do this early when updating nixVersions.git.
This turns out to be a big problem for performance of Bison generated code, that for whatever reason cannot be made internal to the shared library. This causes GCC to make a bunch of function calls go through PLT. Ideally these hot functions (like move/copy ctor) could become inline in upstream Bison. That will make sure that GCC can do interprocedular optimizations without -fno-semantic-interposition [^]. Considering that LLVM already does inlining and whatnot is a good motivation for this change. I don't know of any case where Nix relies on LD_PRELOAD tricks for the shared libraries in production use-cases. [^]: https://maskray.me/blog/2021-05-09-fno-semantic-interposition
Radvendii
reviewed
Sep 29, 2025
Mic92
approved these changes
Sep 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Since the parser is now LALR we can easily switch
over to the less ugly sketelon than the default C one.
This would allow us to switch from %union to %define api.value.type variant
in the future to avoid the need for triviall POD types.
The second commit addresses some of the performance problems that can be
seen on GCC due to piss poor performance of shared objects with interposition
(calls via PLT) because GCC (unlike Clang) doesn't do any inlining of non-inline
functions with default linkage.
This turns out to be a big problem for performance of Bison
generated code, that for whatever reason cannot be made internal
to the shared library. This causes GCC to make a bunch of function
calls go through PLT. Ideally these hot functions (like move/copy ctor) could become
inline in upstream Bison. That will make sure that GCC can do interprocedular
optimizations without
-fno-semantic-interposition^. Considering thatLLVM already does inlining and whatnot is a good motivation for this change.
I don't know of any case where Nix relies on
LD_PRELOADtricks for the sharedlibraries in production use-cases.
Also building with
-fno-semantic-interpositionand-Wl,-Bsymbolic-functionsshould become the norm rather than the exception.
Context
Will be very useful for getting rid of the
%unionand a lot of unnecessary allocations in the parser #14090. This change has also been long awaited since the parser has been made LALR by @rhendric in #11145.Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.