Skip to content

Fix: don't set external linkage when @[NoInline] is specified#15424

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
ysbaddaden:fix/dont-set-external-linkage-to-noinline-def
Feb 7, 2025
Merged

Fix: don't set external linkage when @[NoInline] is specified#15424
straight-shoota merged 1 commit intocrystal-lang:masterfrom
ysbaddaden:fix/dont-set-external-linkage-to-noinline-def

Conversation

@ysbaddaden
Copy link
Collaborator

@ysbaddaden ysbaddaden commented Feb 6, 2025

Unlike alwaysinline that is treated by LLVM as a simple hint, the noinline function attribute is enough to tell LLVM to always generate the symbol and never inline the calls (even at aggressive optimizations).

We don't need to set the linkage to external as well.

Related to #921

Unlike `alwaysinline` that is treated by LLVM as a simple hint, the
`noinline` function attribute is enough to tell LLVM to always generate
the symbol and never inline the calls (even with aggressive
optimizations). We don't need to set the linkage to external.
@ysbaddaden ysbaddaden self-assigned this Feb 6, 2025
@straight-shoota straight-shoota added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Feb 6, 2025
@straight-shoota straight-shoota added this to the 1.16.0 milestone Feb 6, 2025
@straight-shoota straight-shoota merged commit 5172d01 into crystal-lang:master Feb 7, 2025
72 checks passed
@ysbaddaden ysbaddaden deleted the fix/dont-set-external-linkage-to-noinline-def branch February 7, 2025 13:24
@crysbot
Copy link
Collaborator

crysbot commented Oct 30, 2025

This pull request has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/generate-a-library-and-load-it/7570/6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:codegen

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants