Skip to content

Completely remove build_aten and use_aten #10469

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.

Sign up for GitHub

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

Jump to bottom
Closed
mingzhe09088 wants to merge 28 commits into from
Closed

Completely remove build_aten and use_aten #10469

mingzhe09088 wants to merge 28 commits into from

Conversation

@mingzhe09088
Copy link
Contributor

@mingzhe09088 mingzhe09088 commented Aug 13, 2018

Breaking out of #8338 to completely remove build_aten and use_aten.

@mingzhe09088 mingzhe09088 requested a review from orionr August 13, 2018 16:56
orionr
orionr approved these changes Aug 13, 2018
View reviewed changes
Copy link
Contributor

@orionr orionr left a comment

Choose a reason for hiding this comment

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

One thing I'm debating is whether we want all of the ANDROID and IOS checks (if (NOT ANDROID and NOT IOS), etc.), but I think we are likely to incrementally remove these checks over time, so it's okay. I'm guessing all of these are needed for iOS and Android builds to pass?

@mingzhe09088
Copy link
Contributor Author

mingzhe09088 commented Aug 13, 2018

Yes. The checks for mobile is needed for Android and iOS builds to pass.

@ezyang
Copy link
Contributor

ezyang commented Aug 15, 2018

It seems like an un-improvement to inline BUILD_ATEN with NOT ANDROID AND NOT IOS. Maybe it would be better to introduce something like MOBILE.

@bddppq
Copy link
Contributor

bddppq commented Aug 18, 2018

@pytorchbot retest this please

@bddppq
Copy link
Contributor

bddppq commented Aug 18, 2018

https://ci.pytorch.org/jenkins/job/caffe2-builds/job/py2-clang3.8-rocm1.7.1-ubuntu16.04-build/10441/console

06:45:13 [ 91%] Linking HIP shared library ../lib/libcaffe2_hip.so
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_CUDAByteType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_CUDACharType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_CUDADoubleType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_CUDAFloatType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_CUDAGenerator.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_CUDAHalfType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_CUDAIntType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_CUDALongType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_CUDAShortType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_RegisterCUDA.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_SparseCUDAByteType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_SparseCUDACharType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_SparseCUDADoubleType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_SparseCUDAFloatType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_SparseCUDAIntType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_SparseCUDALongType.h.o: File format not recognized
06:45:14 objdump: CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_SparseCUDAShortType.h.o: File format not recognized
07:18:20 CMakeFiles/caffe2_hip.dir/__/build_caffe2/aten/src/ATen/caffe2_hip_generated_CUDAByteType.h.o: file not recognized: File format not recognized
07:18:20 clang-7.0: error: linker command failed with exit code 1 (use -v to see invocation)
07:18:20 Died at /opt/rocm/hip/bin/hipcc line 565.

@iotamudelta @Jorghi12 Could you help taking a look?

@bddppq
Copy link
Contributor

bddppq commented Aug 18, 2018

After investigation this is because ATen_CUDA_SRCS contains header files, and hip_add_library determines whether a file is header only based on property HEADER_FILE_ONLY and not look into the filename. Filtering out the headers from ATen_CUDA_SRCS has fixed the issue.

bddppq
bddppq reviewed Aug 19, 2018
View reviewed changes
aten/src/ATen/CMakeLists.txt Outdated

This comment was marked as off-topic.

Sign in to view

bddppq
bddppq reviewed Aug 20, 2018
View reviewed changes
.jenkins/pytorch/build.sh Outdated

This comment was marked as off-topic.

Sign in to view

bddppq
bddppq reviewed Aug 20, 2018
View reviewed changes
cmake/Dependencies.cmake Outdated

This comment was marked as off-topic.

Sign in to view

@mingzhe09088
Copy link
Contributor Author

mingzhe09088 commented Aug 20, 2018

@pytorchbot retest this please

bddppq and others added 15 commits August 20, 2018 15:52
@bddppq @mingzhe09088
.
7071ecf 
This reverts commit 8f988b6a9225502e9b9c22338ad473aac18e66f8.
@bddppq @mingzhe09088
export USE_ROCM=1
19c7208
@bddppq @mingzhe09088
-DUSE_ROCM
c3f1747
@bddppq @mingzhe09088
Revert "export USE_ROCM=1"
745f99e 
This reverts commit 9b4a4446eeaa09e453a7be5f6294b0c733df86fc.
@bddppq @mingzhe09088
Revert "-DUSE_ROCM"
d3cf8a1 
This reverts commit 9eef9fc8fd1a5497aec7236872d925443b6a47f0.
@bddppq @mingzhe09088
.
b173b7f
@bddppq @mingzhe09088
.
90eb09f
@bddppq @mingzhe09088
ignore unresolved symbols for rocm builds
48d5149
@bddppq @mingzhe09088
.
33549aa
@bddppq @mingzhe09088
.
71e3cf6
@bddppq @mingzhe09088
.
93ac886
@bddppq @mingzhe09088
.
6dd3a4d
@bddppq @mingzhe09088
.
fa28636
@mingzhe09088
introduce BUILD_ATEN_MOBILE
667697e
@mingzhe09088
cleanup
2e93568
facebook-github-bot
facebook-github-bot reviewed Aug 20, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

mingzhe09088 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@bddppq
Copy link
Contributor

bddppq commented Aug 21, 2018

Alt Text

zdevito pushed a commit to zdevito/ATen that referenced this pull request Aug 21, 2018
@mingzhe09088 @facebook-github-bot
Completely remove build_aten and use_aten (#10469)
728a732 
Summary:
Breaking out of #8338 to completely remove build_aten and use_aten.
Pull Request resolved: pytorch/pytorch#10469

Reviewed By: orionr

Differential Revision: D9413639

Pulled By: mingzhe09088

fbshipit-source-id: b7203aa4f5f2bb95c504c8dc187a3167f2570183
petrex pushed a commit to petrex/pytorch that referenced this pull request Aug 21, 2018
Merge remote-tracking branch 'upstream/master' into hipGlooIntegration
f532d9a 
* upstream/master: (147 commits)
  Support Loading to GPU (pytorch#10710)
  More changes for hidden visibility (pytorch#10692)
  Add arguments __repr__ in Distribution base class
  Add util function from core type to dtype (pytorch#10716)
  Make ONNX_ATEN_FALLBACK as internal default option
  Set the BUILD_ENVIRONMENT variable before installing sccache. (pytorch#10640)
  Avoid shadowing i, j vars in GeneralProposals test (pytorch#10721)
  Move THNN Reduction to ATen/core. (pytorch#10703)
  Completely remove build_aten and use_aten (pytorch#10469)
  Make empty list literals construct empty Tensor[] (pytorch#10705)
  Soumith's last few patches to v0.4.1
  Fix issues link in Caffe2 readme (pytorch#10711)
  Update the onnx Gemm op to FC/FCTransposed logic in caffe2 onnx backend (pytorch#10108)
  Adding ATEN_NO_TEST option to root level cmake for propogation to aten
  Allow method-style casts on tensors (pytorch#10641)
  Fix pytorch#10698 build failure (pytorch#10704)
  Add support for Log()
  Add a bisect percentile operator (pytorch#10563)
  Fix EnsureCPUOutputOp (pytorch#10651)
  Nomnigraph - rename some APIs that invole Subtree to Subgraph (pytorch#10551)
  ...
PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request Sep 11, 2018
@mingzhe09088 @PenghuiCheng
Completely remove build_aten and use_aten (pytorch#10469)
6fe219b 
Summary:
Breaking out of pytorch#8338 to completely remove build_aten and use_aten.
Pull Request resolved: pytorch#10469

Reviewed By: orionr

Differential Revision: D9413639

Pulled By: mingzhe09088

fbshipit-source-id: b7203aa4f5f2bb95c504c8dc187a3167f2570183
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@orionr orionr orionr approved these changes

@apaszke apaszke Awaiting requested review from apaszke

@colesbury colesbury Awaiting requested review from colesbury

@ezyang ezyang Awaiting requested review from ezyang

@gchanan gchanan Awaiting requested review from gchanan

@soumith soumith Awaiting requested review from soumith

@zdevito zdevito Awaiting requested review from zdevito

+1 more reviewer

@bddppq bddppq bddppq left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mingzhe09088 @ezyang @bddppq @orionr @facebook-github-bot