[iOS] Add watchOS support #33318
[iOS] Add watchOS support #33318
Conversation
[ghstack-poisoned]
xta0
requested review from
ljk53,
dreiss,
ezyang,
IvanKobzarev and
shoumikhin
CMakeLists.txt
Outdated
| @@ -343,7 +343,9 @@ if (INTERN_BUILD_MOBILE AND NOT BUILD_CAFFE2_MOBILE) | |||
| set(FEATURE_TORCH_MOBILE ON) | |||
| set(NO_API ON) | |||
| set(USE_FBGEMM OFF) | |||
| set(USE_PYTORCH_QNNPACK ON) | |||
| if (NOT USE_PYTORCH_QNNPACK) | |||
There was a problem hiding this comment.
because it'll overwrite this param in iOS build script. See the change in build_ios.sh down below.
There was a problem hiding this comment.
I'm not sure if there is a way to differentiate unset variable v.s. OFF variable in cmake. I think even if you explicitly set USE_PYTORCH_QNNPACK to OFF below, "NOT USE_PYTORCH_QNNPACK" will evaluate to TRUE and it will be set to ON again. I haven't tested, though.
I was thinking of not exposing this USE_PYTORCH_QNNPACK flag to external developer to reduce complexity, as it is always ON for mobile builds. But if there is legit reason to disable it for certain mobile devices, maybe we should just remove set(USE_PYTORCH_QNNPACK ON) here.
There was a problem hiding this comment.
@ljk53 should be if(NOT DEFINED USE_PYTORCH_QNNPACK). My mistake.
### Summary Recently, we have a [discussion](https://discuss.pytorch.org/t/libtorch-on-watchos/69073/14) in the forum about watchOS. This PR adds the support for building watchOS libraries. ### Test Plan - `BUILD_PYTORCH_MOBILE=1 IOS_PLATFORM=WATCHOS ./scripts/build_ios.sh` Differential Revision: [D19896534](https://our.internmc.facebook.com/intern/diff/D19896534) [ghstack-poisoned]
CMakeLists.txt
Outdated
| @@ -343,7 +343,9 @@ if (INTERN_BUILD_MOBILE AND NOT BUILD_CAFFE2_MOBILE) | |||
| set(FEATURE_TORCH_MOBILE ON) | |||
| set(NO_API ON) | |||
| set(USE_FBGEMM OFF) | |||
| set(USE_PYTORCH_QNNPACK ON) | |||
| if (NOT DEFINED USE_PYTORCH_QNNPACK) | |||
There was a problem hiding this comment.
I think we set default value for "USE_PYTORCH_QNNPACK" on Line 181 - will NOT DEFINED ever become true?
I think we should simply removed "set(USE_PYTORCH_QNNPACK ON)" here as we already have default value set to ON for it. I cannot remember why I set this in the first place, maybe was trying to enforce using it everywhere for mobile.
There was a problem hiding this comment.
That's the best way to do it.
### Summary Recently, we have a [discussion](https://discuss.pytorch.org/t/libtorch-on-watchos/69073/14) in the forum about watchOS. This PR adds the support for building watchOS libraries. ### Test Plan - `BUILD_PYTORCH_MOBILE=1 IOS_PLATFORM=WATCHOS ./scripts/build_ios.sh` Differential Revision: [D19896534](https://our.internmc.facebook.com/intern/diff/D19896534) [ghstack-poisoned]
| @@ -343,7 +343,6 @@ if (INTERN_BUILD_MOBILE AND NOT BUILD_CAFFE2_MOBILE) | |||
| set(FEATURE_TORCH_MOBILE ON) | |||
| set(NO_API ON) | |||
| set(USE_FBGEMM OFF) | |||
| set(USE_PYTORCH_QNNPACK ON) | |||
| set(USE_QNNPACK OFF) | |||
There was a problem hiding this comment.
Oh, I think I remember why I added the line above - because I turned USE_QNNPACK OFF and turned USE_PYTORCH_QNNPACK ON - I want to pair them up. i.e. the reason we turned off USE_QNNPACK here is because we turned on better alternative. (just comment, no action needed)
| message(STATUS " USE_QNNPACK : ${USE_QNNPACK}") | ||
| message(STATUS " USE_PYTORCH_QNNPACK : ${USE_PYTORCH_QNNPACK}") |
There was a problem hiding this comment.
I really hope we can consolidate these two one day :) (again, just comment)
💊 CircleCI build failures summary and remediationsAs of commit ac4663f:
Detailed failure analysisOne may explore the probable reasons each build failed interactively on the Dr. CI website. 1 failure not recognized by patterns:
This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker. |
Summary: Pull Request resolved: pytorch#33318 ### Summary Recently, we have a [discussion](https://discuss.pytorch.org/t/libtorch-on-watchos/69073/14) in the forum about watchOS. This PR adds the support for building watchOS libraries. ### Test Plan - `BUILD_PYTORCH_MOBILE=1 IOS_PLATFORM=WATCHOS ./scripts/build_ios.sh` Test Plan: Imported from OSS Differential Revision: D19896534 Pulled By: xta0 fbshipit-source-id: 7b9286475e895d9fefd998246e7090ac92c4c9b6
Stack from ghstack:
Summary
Recently, we have a discussion in the forum about watchOS. This PR adds the support for building watchOS libraries.
Test Plan
BUILD_PYTORCH_MOBILE=1 IOS_PLATFORM=WATCHOS ./scripts/build_ios.shDifferential Revision: D19896534