-
Notifications
You must be signed in to change notification settings - Fork 722
Update XNNPACK to 1ed874e65 #6538
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/6538
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit a8af86b with merge base e965347 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
670a14c to
9d704ab
Compare
.ci/scripts/test_llama.sh
Outdated
| DTYPE=$3 # fp16, bf16, or fp32 | ||
| MODE=${4:-"xnnpack+custom"} # portable or xnnpack+custom or xnnpack+custom+qe | ||
| UPLOAD_DIR=${5:-} | ||
| CMAKE_BUILD_TYPE=${6:-Release} |
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 get this from ENV
c3d4da7 to
90995b9
Compare
|
@mcr229 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| DTYPE=$3 # fp16, bf16, or fp32 | ||
| MODE=${4:-"xnnpack+custom"} # portable or xnnpack+custom or xnnpack+custom+qe | ||
| UPLOAD_DIR=${5:-} | ||
| CMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE:-Release} |
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 do we need make Release the new default?
|
|
||
| # # run e2e (export, tokenizer and runner) | ||
| # PYTHON_EXECUTABLE=python ${CONDA_RUN} bash .ci/scripts/test_llava.sh Release | ||
| # PYTHON_EXECUTABLE=python ${CONDA_RUN} bash .ci/scripts/test_llava.sh |
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.
oh it was Release, keep debug default in the script and pass this as is?
| native.cxx_library( | ||
| name = "subgraph", | ||
| srcs = SUBGRAPH_SRCS, | ||
| srcs = SUBGRAPH_SRCS + ["XNNPACK/src/datatype.c"], |
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.
fix this upstream?
|
@mcr229 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@pytorchbot rebase -s |
|
@mcr229 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@pytorchbot merge |
|
Mergebot is not configured for this repository. Please use the merge button provided by GitHub. |
|
This pull request was exported from Phabricator. Differential Revision: D65563639 |
Summary: As Title Pull Request resolved: pytorch#6538 Reviewed By: cccclai Differential Revision: D65563639 Pulled By: mcr229
Summary: As Title Pull Request resolved: pytorch#6538 Reviewed By: cccclai Differential Revision: D65563639 Pulled By: mcr229
|
This pull request was exported from Phabricator. Differential Revision: D65563639 |
02b1466 to
bc3e4ab
Compare
|
@mcr229 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@mcr229 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@mcr229 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@mcr229 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
looks like this is good to go? failures look like 1 flake, 1 forked PR problem, 1 unclear |
Yeah, they are all unrelated. Let me push a fix for the forked PR as it will always fails |
|
yea sorry i've been trying to land this by rebasing and importing, but from internal it keeps failing because of some phantom merge conflict. |
|
@mcr229 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@mcr229 Let me try to land this, the diff signals complaining about PR cannot be merged looks weird. I think I know a way to bypass it, so let's see |
As Title