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

Adding Function Parameter + Return Attributes #132

Merged
merged 2 commits into from
Jul 10, 2024
Merged

Adding Function Parameter + Return Attributes #132

merged 2 commits into from
Jul 10, 2024

Conversation

jpmedinagl
Copy link
Collaborator

Adding the functions LLVMRustAddFncParamAttr and LLVMRustAddRetFncAttr to RustWrapper.cpp.

This will be used so that later we can link type trees to specific parameters/return value for Enzyme.

For the c->rust wrapper, do I add an implementation for the LLVMRustAddFncParamAttr, LLVMRustAddRetAttr functions?

Also do I need lines 6-7?

@jpmedinagl jpmedinagl requested a review from ZuseZ4 July 10, 2024 14:49
@ZuseZ4
Copy link
Member

ZuseZ4 commented Jul 10, 2024

Thanks!

@ZuseZ4 ZuseZ4 merged commit eba256f into master Jul 10, 2024
9 of 12 checks passed
@ZuseZ4
Copy link
Member

ZuseZ4 commented Jul 13, 2024

Oh actually pub fn LLVMRustAddRetAttr(V: &Value, attr: AttributeKind); here you should have used Attr, like you did in the other function that you added, but I'll fix it in the PR2. The reason is that AttributeKind is one of the enum variant hardcoded by rustc, but we create our own Attributes, containing tt.

Also, you named the Rust version LLVMRustAddRetAttr, and the C version LLVMRustAddRetFncAttr. These two files exist s.t. the linker can link the Rust version against the C version (which wrapps the real, c++ version), so they have to have identical names to allow the linker to link the c implementation against the rust declaration (linkers ignore argument numbers and types, they only go after names).

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