-
Notifications
You must be signed in to change notification settings - Fork 689
Qualcomm AI Engine Direct - LE support #12164
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
base: main
Are you sure you want to change the base?
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/12164
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New Failures, 2 Cancelled JobsAs of commit 229a3f6 with merge base f4d801a ( NEW FAILURES - The following jobs have failed:
CANCELLED JOBS - The following jobs were cancelled. Please retry:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
@pytorchbot label "release notes: qualcomm" |
- SSG2115P | ||
- SSG2125P | ||
- SXR1230P | ||
- SXR1230P (Linux Embedded) |
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 thought SXR1230P is just a SoC similar to others. How is it special compared with the others?
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.
Sorry that I completely missed this. I think it depends on the device meta: like the info dumped from my device cat /firmware/verinfo/ver_info.txt: Aurora.LE.2.3-00040-STD.PROD-1
.
Some SoCs might have LE / LA simultaneously, but I think this one we only provide the LE meta build.
"-t", | ||
"--target", | ||
help="Target platform for deployment", | ||
default="aarch64-android", |
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.
Can we add choices so users know what they can choose?
default="aarch64-android", | |
choices=["aarch64-android", "linux"] | |
default="aarch64-android", |
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.
That's even better, thank you.
Sorry I need to spend a bit more time on this, because we don't have CI to test the pllm model and I'm worried it will cause breakage |
No worries, this PR is also a PoC for IoT (the toolchain will mostly be LE) device users. |
5b51d08
to
f7771ea
Compare
f7771ea
to
76af36b
Compare
there is a lint error and some ci error |
Hi @cccclai, |
Just approve and have it running, not quite sure why it's not triggered automatically... |
Seems like lint error and qnn related tests are failing |
CMAKE_AARCH64="build-android" | ||
BUILD_ANDROID="true" | ||
CMAKE_ANDROID="build-android" | ||
BUILD_OE_LINUX="false" |
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.
If it's default to false, why do we still need to explicitly say --skip-le?
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.
This is used when --no_clean
appears. Sometimes user makes small changes on x86 only (e.g. pybind layer), they could skip the target platform like --skip_la
or --skip_le
.
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.
Can you remind me what la is? le seems to be linux_embedding, if so, let's use the whole name just so it's easier to follow. Another comment is that, can we make le optional? I'd like to avoid patching it everywhere, and it seems like linux embedding isn't super common (?)
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.
la
stands for linux android
and le
will only be functional if user specifies "build.sh --le". It's commonly used in IoT devices, actually customers like Amazon have requirements for us to enable this.
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 see, glad that Amazon is requesting this! Question about the PR, if le will only be functional when users specifybuild.sh --le
, why do we need to explicitly skip -le in setup.py
. Also, a minor comment to use linux_embedding
and linux_android
so it's easier to understand the code
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 is la
in setup.py
and we will rename the variable, thank you for the suggestion.
7afa780
to
902aea7
Compare
I feel like some CI is failing, but I'm not sure if it's related...also static-llama-qnn seems cancelled somehow |
After double check the log, I feel like this PR base commit is still old, can you rebase? |
be0f334
to
d779ac4
Compare
2c80292
to
7e14ae0
Compare
There seems to be an error
|
b29b46d
to
5f21d9a
Compare
Hi @cccclai, |
Ah yeah, it was failing in main last week. I think rebasing should fix the issue |
Summary: - enable linux embedded support for toolchain: aarch64-oe-linux-gcc9.3, aarch64-oe-linux-gcc11.2 (SXR1230P, QCS9100) - update document a bit
5f21d9a
to
229a3f6
Compare
Summary
Test plan