Skip to content

Conversation

AMS21
Copy link
Contributor

@AMS21 AMS21 commented Oct 7, 2025

Add LLVMGetOrInsertFunction to the C API as a thin wrapper over Module::getOrInsertFunction, upstreamed from rustc.
It provides a single-call way to get or create a function declaration, avoiding LLVMGetNamedFunction + LLVMAddFunction and is more idiomatic.

@AMS21 AMS21 requested a review from nikic as a code owner October 7, 2025 07:29
@AMS21
Copy link
Contributor Author

AMS21 commented Oct 7, 2025

As I'm not very familiar with the llvm-c codebase I'm not sure where to add tests for the new function. Any help or directions would be appreciated :)

@nikic
Copy link
Contributor

nikic commented Oct 7, 2025

As I'm not very familiar with the llvm-c codebase I'm not sure where to add tests for the new function. Any help or directions would be appreciated :)

Either llvm/tools/llvm-c-test or llvm/unittests/IR/FunctionTest.cpp would work. Probably the latter in this case (see the last test function there).

@AMS21 AMS21 force-pushed the upstream_get_or_insert_function branch from 59cc9ec to aa16c22 Compare October 7, 2025 13:01
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add an entry to llvm/docs/ReleaseNotes.md and add a proper PR description?

Otherwise this looks like. It's already possible to achieve this using LLVMAddFunction + LLVMGetNamedFunction, but getOrInsertFunction() is generally the more idiomatic way to insert declarations, so it makes sense to me to provide it directly.

@AMS21 AMS21 force-pushed the upstream_get_or_insert_function branch from aa16c22 to 6539a2d Compare October 7, 2025 13:33
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Add `LLVMGetOrInsertFunction` to the C API as a thin wrapper over
`Module::getOrInsertFunction`, upstreamed from rustc
(https://github.com/rust-lang/rust/blob/d773bd07d63a74adcf25ea5f4aae986be94cac5e/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp#L203)
It provides a single-call way to get or create a function declaration,
avoiding `LLVMGetNamedFunction` + `LLVMAddFunction` and is more idiomatic.
@AMS21 AMS21 force-pushed the upstream_get_or_insert_function branch from 6539a2d to 756bae5 Compare October 7, 2025 14:17
@nikic nikic enabled auto-merge (squash) October 7, 2025 14:18
@nikic nikic merged commit 01f4510 into llvm:main Oct 7, 2025
10 checks passed
@AMS21 AMS21 deleted the upstream_get_or_insert_function branch October 7, 2025 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants