Skip to content

[BE] Make Vec256 header only library#50708

Closed
malfet wants to merge 1 commit intomasterfrom
malfet/make-vec256-header-library-again
Closed

[BE] Make Vec256 header only library#50708
malfet wants to merge 1 commit intomasterfrom
malfet/make-vec256-header-library-again

Conversation

@malfet
Copy link
Copy Markdown
Contributor

@malfet malfet commented Jan 19, 2021

Do it by removing extraneous header dependencies.
None of the at::vec256 primitive depend on notion of Tensor, therefore none of the headers that vec256 depends on should include <ATen/Tensor.h>

Implicity test it be removing c10 and tensor dependency when building vec256_test_all_types
Split affine_quantizer into affine_quantizer_base (that contains methods operating on raw types) and affine_quantizer (which contains Tensor specific methods)

Fixes #50567

@malfet malfet requested a review from a team January 19, 2021 02:48
Copy link
Copy Markdown
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.

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

Comment thread caffe2/CMakeLists.txt Outdated
@malfet malfet force-pushed the malfet/make-vec256-header-library-again branch from 1599359 to ee1de8a Compare January 20, 2021 03:51
Copy link
Copy Markdown
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.

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

@malfet malfet force-pushed the malfet/make-vec256-header-library-again branch from ee1de8a to 8fbb9df Compare January 20, 2021 04:58
Copy link
Copy Markdown
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.

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

@malfet malfet force-pushed the malfet/make-vec256-header-library-again branch from 8fbb9df to b4bfe61 Compare January 20, 2021 14:36
Copy link
Copy Markdown
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.

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

@malfet malfet force-pushed the malfet/make-vec256-header-library-again branch 6 times, most recently from b59213f to dbf9d30 Compare January 21, 2021 19:44
Do it by removing extraneous header dependencies.
None of the at::vec256 primitive depend on notion of Tensor, therefore none of the headers that vec256 depends on should include <ATen/Tensor.h>

Implicity test it be removing c10 and tensor dependency when building `vec256_test_all_types`
Split affine_quantizer into affine_quantizer_base (that contains methods operating on raw types) and affine_quantizer (which contains Tensor specific methods)
@malfet malfet force-pushed the malfet/make-vec256-header-library-again branch from dbf9d30 to cd4afd4 Compare January 23, 2021 01:29
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 23, 2021

Codecov Report

Merging #50708 (cd4afd4) into master (789f6f1) will decrease coverage by 0.00%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##           master   #50708      +/-   ##
==========================================
- Coverage   80.97%   80.97%   -0.01%     
==========================================
  Files        1919     1920       +1     
  Lines      209785   209785              
==========================================
- Hits       169875   169872       -3     
- Misses      39910    39913       +3     

Copy link
Copy Markdown
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.

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@malfet merged this pull request in 48b6b92.

@malfet malfet deleted the malfet/make-vec256-header-library-again branch September 23, 2021 18:12
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Do it by removing extraneous header dependencies.
None of the at::vec256 primitive depend on notion of Tensor, therefore none of the headers that vec256 depends on should include <ATen/Tensor.h>

Implicity test it be removing c10 and tensor dependency when building `vec256_test_all_types`
Split affine_quantizer into affine_quantizer_base (that contains methods operating on raw types) and affine_quantizer (which contains Tensor specific methods)

Fixes pytorch#50567

Pull Request resolved: pytorch#50708

Reviewed By: walterddr

Differential Revision: D25949168

Pulled By: malfet

fbshipit-source-id: c3323be7252865a52c7d94026a5a39b494e44efb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ATen/cpu/vec256/ should be a header only library for all but quantized types

3 participants