Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose TargetLibraryInfo pass #934

Merged
merged 4 commits into from May 18, 2023
Merged

Conversation

sklam
Copy link
Member

@sklam sklam commented Apr 14, 2023

Needed for numba/numba#8898

@sklam sklam marked this pull request as ready for review April 25, 2023 16:09
@esc esc requested review from apmasell and esc May 2, 2023 14:12
ffi/targets.cpp Outdated
LLVMPassManagerRef PM) {
unwrap(PM)->add(new TargetLibraryInfoWrapperPass(*unwrap(TLI)));
API_EXPORT(void)
LLVMPY_AddTargetLibraryInfoImpl(const char *TripleStr, LLVMPassManagerRef PM) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this Impl? Most of the other passes have Pass as a suffix.

Copy link
Member Author

Choose a reason for hiding this comment

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

changed in 1880582

@@ -911,6 +914,8 @@ def run_with_remarks(self, function, remarks_format='yaml',
ffi.lib.LLVMPY_AddSROAPass.argtypes = [ffi.LLVMPassManagerRef]
ffi.lib.LLVMPY_AddTypeBasedAliasAnalysisPass.argtypes = [ffi.LLVMPassManagerRef]
ffi.lib.LLVMPY_AddBasicAliasAnalysisPass.argtypes = [ffi.LLVMPassManagerRef]
ffi.lib.LLVMPY_AddTargetLibraryInfoImpl.argtypes = [c_char_p,
Copy link
Contributor

Choose a reason for hiding this comment

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

All of the other pass functions take the pass manager as the first argument. Please swap the arguments to be consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in 1880582

- Rename LLVMPY_AddTargetLibraryInfoImpl to LLVMPY_AddTargetLibraryInfoPass.
- Reorder it's arguments to match other similar functions.
@sklam sklam merged commit ffcd842 into numba:main May 18, 2023
18 checks passed
@sklam sklam deleted the enh/targetlibraryinfo branch May 18, 2023 20:02
@sklam sklam added this to the v0.41.0 milestone May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants