Plumbing for dtype selection in cmake flow - header generation #11432
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11432
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Merge Blocking SEVsThere is 1 active merge blocking SEVs. Please view them below:
If you must merge, use ❌ 2 New Failures, 2 Unrelated FailuresAs of commit 76cc4d4 with merge base 439aab7 ( NEW FAILURES - The following jobs have failed:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
@pytorchbot label "release notes: none" |
pytorch-bot
bot
added
the
release notes: none
BujSet
requested review from
jathu,
larryliu0820,
kirklandsign,
lucylq and
JacobSzwejbka
as code owners
| cmake_parse_arguments(GEN "" "" "${arg_names}" ${ARGN}) | ||
|
|
||
| message(STATUS "Generating operator lib:") | ||
| message(STATUS " LIB_NAME: ${GEN_LIB_NAME}") | ||
| message(STATUS " OPS_SCHEMA_YAML: ${GEN_OPS_SCHEMA_YAML}") | ||
| message(STATUS " ROOT_OPS: ${GEN_ROOT_OPS}") | ||
| message(STATUS " INCLUDE_ALL_OPS: ${GEN_INCLUDE_ALL_OPS}") | ||
| message(STATUS " DTYPE_SELECT: ${GEN_DTYPE_SELECT}") |
There was a problem hiding this comment.
Where is GEN_DTYPE_SELECT being defined?
There was a problem hiding this comment.
GEN_DTYPE_SELECT is toggled ON or OFF via the command line switch -DEXECUTORCH_SELECT_DTYPE=ON. The option is defined in examples/selective_build/CMakeLists.txt.
| @@ -108,16 +112,30 @@ gen_selected_ops( | |||
| "${EXECUTORCH_SELECT_OPS_LIST}" | |||
| INCLUDE_ALL_OPS | |||
| "${EXECUTORCH_SELECT_ALL_OPS}" | |||
| DTYPE_SELECT | |||
There was a problem hiding this comment.
nit: could we use DTYPE_SELECTIVE_BUILD to mirror the buck API here?
| "${_custom_ops_yaml}" | ||
| DTYPE_SELECT |
There was a problem hiding this comment.
I think we only need this option in gen_selected_ops at the moment, so let's leave it out of the other APIs for now?
There was a problem hiding this comment.
I was running into an issue where if the selected_op_variants.h file wasn't include in the sources list in the gen_operators_lib function, cmake was smart enough to delete it. The casing in that function conditionally adds it to the source list if the flag is set.
| @@ -94,6 +94,7 @@ test_cmake_select_all_ops() { | |||
| rm -rf ${build_dir} | |||
| retry cmake -DCMAKE_BUILD_TYPE=Release \ | |||
| -DEXECUTORCH_SELECT_ALL_OPS=ON \ | |||
| -DEXECUTORCH_SELECT_DTYPE=ON \ | |||
There was a problem hiding this comment.
Let's add these in once the E2E is working - currently, it won't enable dtype selective build so may be misleading
| @@ -78,6 +98,13 @@ function(generate_bindings_for_kernels) | |||
| # By default selective build output is selected_operators.yaml | |||
| set(_oplist_yaml ${_out_dir}/selected_operators.yaml) | |||
|
|
|||
| # If dtype selective build is enable, force header file to be preserved | |||
| if(GEN_DTYPE_SELECT) | |||
There was a problem hiding this comment.
We can leave this out for now, as it'll be preserved once we link the header into the portable library.
Summary
This change introduces a new
cmakecommand line option that can be enable via-DEXECUTORCH_SELECT_DTYPE=ON.For now , enabling this flag does not change the generated binary size. This change simply add the header file generation to the cmake build flow. Next steps will include automatically have the cmake build flow transplant the contents of the generated header file directly into thekernels/portable/cpu/selective_build.hfile. This step must manually be done by a user to enable directed dtype pruning.Test plan
Changes were test via
bash examples/selective_build/test_selective_build.sh. The resultant header file generated by the flow is now created and retained atcmake-out/examples/selective_build/select_build_lib/selected_op_variants.h.