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

[OM] Add initial CAPI for OM dialect Evaluator. #5248

Merged
merged 3 commits into from
May 24, 2023

Conversation

mikeurbach
Copy link
Contributor

This adds the usual CAPI structure and dialect registration boilerplate, as well as CAPIs around the Evaluator library.

The APIs are intended to be as minimal and straightforward as possible, simply wrapping and unwrapping the C++ structures when possible.

One slight divergence is in the ObjectValue type, which is a std::variant between an Object shared pointer or an Attribute. The normal approach of casting to and from a void pointer does not work with std::variant, so a different representation of ObjectValue is used instead. The discriminated union is simply represented as a struct which only over has one field set. It might be possible to save some space using a struct with a C union and a flag, but the simplicity of the current approach seemed reasonable.

Another minor detail worth mentioning is that we must take some care to ensure the shared pointers to Objects have their reference count kept up to date. In the CAPI for the instantiate method, if we simply return the shared pointer, the reference will be lost as the Object pointer travels to C as a void pointer, so we allocate a new shared pointer in the CAPI, which ensures the reference count accurately reflect that we have handed out another reference.

This adds the usual CAPI structure and dialect registration
boilerplate, as well as CAPIs around the Evaluator library.

The APIs are intended to be as minimal and straightforward as
possible, simply wrapping and unwrapping the C++ structures when
possible.

One slight divergence is in the ObjectValue type, which is a
std::variant between an Object shared pointer or an Attribute. The
normal approach of casting to and from a void pointer does not work
with std::variant, so a different representation of ObjectValue is
used instead. The discriminated union is simply represented as a
struct which only over has one field set. It might be possible to save
some space using a struct with a C union and a flag, but the
simplicity of the current approach seemed reasonable.

Another minor detail worth mentioning is that we must take some care
to ensure the shared pointers to Objects have their reference count
kept up to date. In the CAPI for the instantiate method, if we simply
return the shared pointer, the reference will be lost as the Object
pointer travels to C as a void pointer, so we allocate a new shared
pointer in the CAPI, which ensures the reference count accurately
reflect that we have handed out another reference.
Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@prithayan prithayan left a comment

Choose a reason for hiding this comment

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

Looks good.

@mikeurbach mikeurbach merged commit 56861ea into main May 24, 2023
@mikeurbach mikeurbach deleted the mikeurbach/om-dialect-capi branch May 24, 2023 15:30
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.

3 participants