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

[onert] Introduce CodegenManager class #12553

Merged
merged 2 commits into from
Jan 31, 2024

Conversation

jyoungyun
Copy link
Contributor

This commit introduces CodegenManager class.
This class runs codegen() function to load target backend library and call the codegen function.

ONE-DCO-1.0-Signed-off-by: Jiyoung Yun jy910.yun@samsung.com
Co-authored-by: Sanggyu Lee sg5.lee@samsung.com

Related issue : #12505
Draft PR: #12504

This commit introduces CodegenManager class.
This class runs codegen() function to load target backend library
and call the codegen function.

ONE-DCO-1.0-Signed-off-by: Jiyoung Yun <jy910.yun@samsung.com>
Co-authored-by: Sanggyu Lee <sg5.lee@samsung.com>
@jyoungyun jyoungyun added the PR/ready for review It is ready to review. Please review it. label Jan 30, 2024
@jyoungyun jyoungyun requested a review from a team January 30, 2024 04:21
runtime/onert/core/src/odc/CodegenManager.cc Outdated Show resolved Hide resolved
namespace odc
{

bool CodegenManager::codegen(const char *target, CodegenPreference pref)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what target is but code that is checking the target is necessary, I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

target is the term in compiler. source and target. You may think it as the binary of codegen. I agree to add check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can find the detail explanation of target string in header file.

   * @param target  Target backend name
   *                This target string will be used to find a backend library.
   *                The name of target backend library should follow the following rules:
   *                  'lib' + {backend extension} + '-gen' + {lib extension}
   *                And the target string should be a name except 'lib' and {lib extension}.
   *                For example, if the backend extension is 'aaa', the backend library name
   *                should be 'libaaa-gen.so', and the target string should be 'aaa-gen'.

This Codegen manager dynamically loads the target library. To find a shared library, a search policy must be defined. In this case, the library is searched for using this target string.

runtime/onert/core/src/odc/CodegenManager.cc Show resolved Hide resolved
runtime/onert/core/src/odc/CodegenManager.cc Outdated Show resolved Hide resolved
ONE-DCO-1.0-Signed-off-by: Jiyoung Yun <jy910.yun@samsung.com>
@jyoungyun
Copy link
Contributor Author

PTAL all :)

Copy link
Contributor

@ragmani ragmani 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

@YongseopKim YongseopKim 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

@glistening glistening left a comment

Choose a reason for hiding this comment

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

LGTM

@glistening glistening merged commit d2ce5d2 into Samsung:master Jan 31, 2024
9 checks passed
@jyoungyun jyoungyun deleted the onert/introduce_codegen_manager branch February 1, 2024 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/ready for review It is ready to review. Please review it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants