-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[microTVM][ARM][Zephyr] Add CMSIS dependencies in Zephyr project build #11362
Conversation
6da51cb
to
8828561
Compare
6fe730c
to
e4035c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @mehrdadh , couple questions
def _load_cmsis(self, lib_path: Union[str, pathlib.Path]): | ||
"""Copy CMSIS header files to generated project.""" | ||
|
||
cmsis_path = pathlib.Path(os.environ["CMSIS_PATH"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not make this a project option for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that should be a project options that I missed. Done!
@@ -455,6 +476,15 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec | |||
os.makedirs(extract_path) | |||
tf.extractall(path=extract_path) | |||
|
|||
# Add CMSIS libraries if required. | |||
if options["project_type"] == "host_driven": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why only for host-driven?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to support both and different module names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PTKL, thanks!
def _load_cmsis(self, lib_path: Union[str, pathlib.Path]): | ||
"""Copy CMSIS header files to generated project.""" | ||
|
||
cmsis_path = pathlib.Path(os.environ["CMSIS_PATH"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that should be a project options that I missed. Done!
@@ -455,6 +476,15 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec | |||
os.makedirs(extract_path) | |||
tf.extractall(path=extract_path) | |||
|
|||
# Add CMSIS libraries if required. | |||
if options["project_type"] == "host_driven": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to support both and different module names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @mehrdadh ! one final question
disabled conv2d_nhwc_dsp.arm_cpu for non integers workloads added debugging feature to TempDirectory
f46fce9
to
b31a618
Compare
@@ -70,6 +69,8 @@ | |||
|
|||
ZEPHYR_BASE = os.getenv("ZEPHYR_BASE") | |||
|
|||
CMSIS_PATH = os.getenv("CMSIS_PATH") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, used it as default if project option is not passed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it might be good to avoid hidden env variable dependencies. they can be hard to track down. what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense, I removed the default value.
def pytest_configure(config): | ||
config.addinivalue_line( | ||
"markers", | ||
"skip_by_board(board): skip test for the given board", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mehrdadh does this need to be skip_boards?
"skip_boards(board): skip test for the given board",
This PR does the following:
cc @areusch @gromero