[XNNPACK] support building xnnpack EP for IOS#13461
Conversation
wejoncy
changed the title
wejoncy
changed the title
| --parallel | ||
| displayName: (CoreML EP) Build onnxruntime for iOS x86_64 and run tests using simulator | ||
|
|
||
| - job: iOS_CI_XNNPACK_on_Mac |
There was a problem hiding this comment.
Do we really need to run this job for every pull request? We have limited macOS machines available, and this build pipeline already takes long.
There was a problem hiding this comment.
What if this job will be running when we trigged it or merged it into main branch
There was a problem hiding this comment.
If ORT has 10 cmake options, then there would be 2^10=1024 different combinations. Our build pipelines can't cover all of them, and we have far more than 10 build options(or probably more than 100). So I suggest focusing on the things we ship. For example, if every iOS package we release contains this XNNPack EP, then we may consider to adjust our pull request pipelines to match it. Meanwhile, you may have extra pipelines to test the combinations that you think are important, but because of resource limitations we can't trigger all such pipelines for all pull requests.
There was a problem hiding this comment.
In this way, maybe we can combine multi build pipelines into one for pull-request. Take IOS-build as example, we can only build once for (CPU-ep,CORE-ep,XNNPACK-ep), How do you think?
When a PR merged, all seperate pipelines are triggered at that time.
| variables: | ||
| MACOSX_DEPLOYMENT_TARGET: '10.14' | ||
| timeoutInMinutes: 100 | ||
| condition: or(eq('${{parameters.BuildXnnpack}}', 'true'), eq(variables['Build.SourceBranch'], 'refs/heads/main')) |
There was a problem hiding this comment.
use template instead.
Like:
${{ if eq(parameters.BuildXnnpack, 'true') }}:
Such statements are evaluated earlier than what you have. "condition" is evaluated at runtime.
There was a problem hiding this comment.
There was a problem hiding this comment.
Sorry, I probably gave you a wrong suggestion. How about this: you move this job to a template file, then include that template file from here. Like
- ${{ if eq(parameters.enable_ubuntu_cpu, true) }}:
- template: py-linux-ubuntu.yml
parameters:
arch: 'x86_64'Then you create another pipeline that will be auto-triggered by every commit in the main branch, but not PRs. Then that pipeline's yaml file include this template too. In this way, your mac-iso-ci-pipeline could still choose to run both jobs or just one of them.
There was a problem hiding this comment.
If you don't want to do that, your previous commit is fine too.
There was a problem hiding this comment.
If I want to control it with both triggerd by mannuly and merged into main, I have to evaluated in the runtime. Because I can only get the parameters.BuildXnnpack when I set it as true, Is that right?
There was a problem hiding this comment.
No, it is not right. parameters are known when you submitted the build run. But at that time, most variables are still unknown. "runtime" means something is only known when the build step starts to run.
| --path_to_protoc_exe $(Build.BinariesDirectory)/protobuf_install/bin/protoc \ | ||
| --parallel | ||
| displayName: (CoreML EP) Build onnxruntime for iOS x86_64 and run tests using simulator | ||
| - ${{ or(if eq(parameters.BuildXnnpack, 'true'), if eq(variables['Build.SourceBranch'], 'refs/heads/main')) }}: |
There was a problem hiding this comment.
We won't know if it works until you merge this PR.
There was a problem hiding this comment.
Now, we know,
-
parameters.BuildXnnpackit works, I have tested it.
-
- when
variables['Build.SourceBranch']is not equal to'refs/heads/main', it doesn't build
- when
There was a problem hiding this comment.
I can test it by merging it into another branch, and have the condition of comparing it to the target branch.
This reverts commit a18fbd4.
### Description support building xnnpack for IOS ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. -->
### Description support building xnnpack for IOS ### Motivation and Context <!-- - Why is this change required? What problem does it solve? - If it fixes an open issue, please link to the issue here. -->
2 participants
Description
support building xnnpack for IOS
Motivation and Context