Skip to content
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

cmake flatcc build issue #4541

Merged
merged 6 commits into from
Aug 8, 2024

Conversation

haowhsu-quic
Copy link
Collaborator

Identical change to #4312.

Copy link

pytorch-bot bot commented Aug 3, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/4541

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 38608a4 with merge base de300e0 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 3, 2024
@cccclai
Copy link
Contributor

cccclai commented Aug 5, 2024

Looks like the coreml delegate test failure is legit, see log


2024-08-04T00:04:12.0402250Z Ld /Users/runner/Library/Developer/Xcode/DerivedData/coreml_executor_runner-bkoiqshilmuvptcxjvpygyufxpnf/Build/Intermediates.noindex/InstallIntermediates/coreml_executor_runner-macosx/InstallableProducts/usr/local/bin/coreml_executor_runner normal (in target 'coreml_executor_runner' from project 'coreml_executor_runner')
--
  | 2024-08-04T00:04:12.0403920Z     cd /Users/runner/work/executorch/executorch/pytorch/executorch/examples/apple/coreml/executor_runner
  | 2024-08-04T00:04:12.0416230Z     /Applications/Xcode_15.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -Xlinker -reproducible -target arm64-apple-macos13.0 -isysroot /Applications/Xcode_15.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk -O0 -L/Users/runner/Library/Developer/Xcode/DerivedData/coreml_executor_runner-bkoiqshilmuvptcxjvpygyufxpnf/Build/Intermediates.noindex/InstallIntermediates/coreml_executor_runner-macosx/Intermediates.noindex/EagerLinkingTBDs/Debug -L/Users/runner/work/executorch/executorch/pytorch/executorch/examples/apple/coreml/xcode-build/Debug -L/Users/runner/work/executorch/executorch/pytorch/executorch/examples/apple/coreml/executor_runner/libraries -L/Users/runner/work/executorch/executorch/pytorch/executorch/examples/apple/coreml/executor_runner/libraries -F/Users/runner/Library/Developer/Xcode/DerivedData/coreml_executor_runner-bkoiqshilmuvptcxjvpygyufxpnf/Build/Intermediates.noindex/InstallIntermediates/coreml_executor_runner-macosx/Intermediates.noindex/EagerLinkingTBDs/Debug -F/Users/runner/work/executorch/executorch/pytorch/executorch/examples/apple/coreml/xcode-build/Debug -filelist /Users/runner/Library/Developer/Xcode/DerivedData/coreml_executor_runner-bkoiqshilmuvptcxjvpygyufxpnf/Build/Intermediates.noindex/InstallIntermediates/coreml_executor_runner-macosx/Intermediates.noindex/coreml_executor_runner.build/Debug/coreml_executor_runner.build/Objects-normal/arm64/coreml_executor_runner.LinkFileList -Xlinker -object_path_lto -Xlinker /Users/runner/Library/Developer/Xcode/DerivedData/coreml_executor_runner-bkoiqshilmuvptcxjvpygyufxpnf/Build/Intermediates.noindex/InstallIntermediates/coreml_executor_runner-macosx/Intermediates.noindex/coreml_executor_runner.build/Debug/coreml_executor_runner.build/Objects-normal/arm64/coreml_executor_runner_lto.o -Xlinker -export_dynamic -Xlinker -no_deduplicate -fobjc-arc -fobjc-link-runtime -all_load -letdump -lexecutorch_no_prim_ops -lflatccrt -lcoremldelegate -framework Accelerate -lprotobuf-lite -lportable_ops_lib -framework CoreML -lportable_kernels -lsqlite3 -lexecutorch -Xlinker -no_adhoc_codesign -Xlinker -dependency_info -Xlinker /Users/runner/Library/Developer/Xcode/DerivedData/coreml_executor_runner-bkoiqshilmuvptcxjvpygyufxpnf/Build/Intermediates.noindex/InstallIntermediates/coreml_executor_runner-macosx/Intermediates.noindex/coreml_executor_runner.build/Debug/coreml_executor_runner.build/Objects-normal/arm64/coreml_executor_runner_dependency_info.dat -o /Users/runner/Library/Developer/Xcode/DerivedData/coreml_executor_runner-bkoiqshilmuvptcxjvpygyufxpnf/Build/Intermediates.noindex/InstallIntermediates/coreml_executor_runner-macosx/InstallableProducts/usr/local/bin/coreml_executor_runner
  | 2024-08-04T00:04:12.0427680Z ld: library 'flatccrt' not found
  | 2024-08-04T00:04:12.0428290Z clang: error: linker command failed with exit code 1 (use -v to see invocation)
  | 2024-08-04T00:04:12.0428630Z
  | 2024-08-04T00:04:12.0428690Z ** BUILD FAILED **
  | 2024-08-04T00:04:12.0428810Z
  | 2024-08-04T00:04:12.0428980Z
  | 2024-08-04T00:04:12.0429140Z The following build commands failed:
  | 2024-08-04T00:04:12.0430710Z 	Ld /Users/runner/Library/Developer/Xcode/DerivedData/coreml_executor_runner-bkoiqshilmuvptcxjvpygyufxpnf/Build/Intermediates.noindex/InstallIntermediates/coreml_executor_runner-macosx/InstallableProducts/usr/local/bin/coreml_executor_runner normal (in target 'coreml_executor_runner' from project 'coreml_executor_runner')
  | 2024-08-04T00:04:12.0432030Z (1 failure)
  | 2024-08-04T00:04:12.0432850Z cp: /Users/runner/work/executorch/executorch/pytorch/executorch/examples/apple/coreml/xcode-build/DEBUG/coreml_executor_runner: No such file or directory
  | 2024-08-04T00:04:12.0433630Z ExecuTorch: Failed to build executor.
  | 2024-08-04T00:04:12.0434210Z ERROR conda.cli.main_run:execute(47): `conda run bash backends/apple/coreml/scripts/build_all.sh` failed. (See above for error)
  | 2024-08-04T00:04:12.0434910Z     main()
  | 2024-08-04T00:04:12.0435510Z   File "/Users/runner/work/executorch/executorch/test-infra/.github/scripts/run_with_env_secrets.py", line 61, in main
  | 2024-08-04T00:04:12.0436520Z     run_cmd_or_die(f"bash { os.environ.get('RUNNER_TEMP', '') }/exec_script")
  | 2024-08-04T00:04:12.0437410Z   File "/Users/runner/work/executorch/executorch/test-infra/.github/scripts/run_with_env_secrets.py", line 39, in run_cmd_or_die
  | 2024-08-04T00:04:12.0438160Z     raise RuntimeError(f"Command {cmd} failed with exit code {exit_code}")
  | 2024-08-04T00:04:12.0438770Z RuntimeError: Command bash /Users/runner/work/_temp/exec_script failed with exit code 1
  | 2024-08-04T00:04:12.0489710Z ##[error]Process completed with exit code 1.
  | 2024-08-04T00:04:12.0599850Z ##[group]Run pmeier/pytest-results-action@v0.3.0

@cccclai
Copy link
Contributor

cccclai commented Aug 5, 2024

Do you have a mac machine to repro?

@haowhsu-quic
Copy link
Collaborator Author

I wonder if this job is making debug build? Since coreml explicitly requires release library.
We'll try to get a mac machine for further investigation, thank you.

@haowhsu-quic
Copy link
Collaborator Author

Could you help make this file into release build (or at least -DCMAKE_BUILD_TYPE=RelWithDebInfo)?

I think this might be the root cause in following log coming from here:
image

@chiwwang
Copy link
Collaborator

chiwwang commented Aug 6, 2024

[Update]
The build order of add_custom_command(OUTPUT ${_etdump_schema__outputs} ...) and add_subdirectory(${_flatcc_source_dir} .... seems not clear if EXECUTORCH_SEPARATE_FLATCC_HOST_PROJECT=ON.

CoreML seems to build with EXECUTORCH_SEPARATE_FLATCC_HOST_PROJECT=ON...?
So, if the below sequence occurs, the flatcc libraries might be deleted by add_custom_command(...${_etdump_schema__outputs}...):

  1. In CMake configure time, the host flatcc is built by execute_process.
  2. In Build time, the add_custom_command(...${_etdump_schema__outputs}...) and flatcc(on target) seems to race. The flatcc libraries built by add_subdirectory(${_flatcc_source_dir} .... might be deleted by add_custom_command(... COMMAND rm -f ${_etdump_schema_cleanup_paths}...)

Need to find a Mac to try....

BTW, another way to resolve the race-condition between the target and the host environment: #4559, which might have smaller changes in CMakeList.
But still, need to find a Mac to try....

@cccclai
Copy link
Contributor

cccclai commented Aug 7, 2024

Catching up....which part makes you feel like release build is the issue here?

@haowhsu-quic
Copy link
Collaborator Author

haowhsu-quic commented Aug 7, 2024

Hi, I add -DCMAKE_BUILD_TYPE=Release to this script. Coreml explicitly requests release version of flatccrt, but the trunk job has no way to set this configuration (backends/apple/scripts/build_all.sh).
I change it with latest commit, all jobs are working properly.

I think why the workflow could work before because the deletion of add_custom_command doesn't really happen (VERBATIM flag introduces quote mark which makes rm fail). The release build library could still be found under artifact directory.

@chiwwang
Copy link
Collaborator

chiwwang commented Aug 7, 2024

Hi, I add -DCMAKE_BUILD_TYPE=Release to this script. Coreml explicitly requests release version of flatccrt, but the trunk job has no way to set this configuration (backends/apple/scripts/build_all.sh). I change it with latest commit, all jobs are working properly now.

Just trying to clarify.
Is the path $EXECUTORCH_ROOT_PATH/third-party/flatcc/lib/libflatccrt.a dedicated for release-build?
I guess if flatcc is not release-built, the libflatccrt.a would be in another place. Is this part making you feel like release build is the issue?

@haowhsu-quic
Copy link
Collaborator Author

haowhsu-quic commented Aug 7, 2024

Hi, I add -DCMAKE_BUILD_TYPE=Release to this script. Coreml explicitly requests release version of flatccrt, but the trunk job has no way to set this configuration (backends/apple/scripts/build_all.sh). I change it with latest commit, all jobs are working properly now.

Just trying to clarify. Is the path $EXECUTORCH_ROOT_PATH/third-party/flatcc/lib/libflatccrt.a dedicated for release-build? I guess if flatcc is not release-built, the libflatccrt.a would be in another place. Is this part making you feel like release build is the issue?

For debug-build library name will be libflatccrt_d.a. We also reproduced the error internally.

@cccclai
Copy link
Contributor

cccclai commented Aug 7, 2024

cc: @Olivia-liu @tarun292 @dbort it really should be our job to make sure flatcc can be built correctly, and thanks folks from Qualcomm helping this.

Copy link
Contributor

@cccclai cccclai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, will wait a bit for the response.

@@ -29,6 +29,7 @@ rm -rf "$CMAKE_BUILD_DIR_PATH"
# Build executorch
echo "ExecuTorch: Building executorch"
cmake "$EXECUTORCH_ROOT_PATH" -B"$CMAKE_BUILD_DIR_PATH" \
-DCMAKE_BUILD_TYPE=Release \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cymbalrush for the change in the coreml script

@Olivia-liu
Copy link
Contributor

cc: @Olivia-liu @tarun292 @dbort it really should be our job to make sure flatcc can be built correctly, and thanks folks from Qualcomm helping this.

Agreed. I will make sure to allocate time to improve the build process for flatcc.

@facebook-github-bot
Copy link
Contributor

@cccclai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot merged commit 2718dd4 into pytorch:main Aug 8, 2024
88 of 89 checks passed
@haowhsu-quic haowhsu-quic deleted the dev_cmake_flatcc branch August 9, 2024 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants