Skip to content

Use templates instead of macro when defining Vec256<BFloat16> bin operators#35844

Closed
xuhdev wants to merge 5 commits intogh/xuhdev/69/basefrom
gh/xuhdev/69/head
Closed

Use templates instead of macro when defining Vec256<BFloat16> bin operators#35844
xuhdev wants to merge 5 commits intogh/xuhdev/69/basefrom
gh/xuhdev/69/head

Conversation

@xuhdev
Copy link
Copy Markdown
Collaborator

@xuhdev xuhdev commented Apr 1, 2020

Stack from ghstack:

Also, bitwise operators can operate on the underlying __m256i
representation directly instead of making expensive conversions to
float16.

Differential Revision: D20927639

…rators

Also, bitwise operators can operate on the underlying __m256i
representation directly instead of making expensive conversions to
float16.

[ghstack-poisoned]
xuhdev added a commit that referenced this pull request Apr 1, 2020
…rators

Also, bitwise operators can operate on the underlying __m256i
representation directly instead of making expensive conversions to
float16.

ghstack-source-id: 83304a6
Pull Request resolved: #35844
@xuhdev xuhdev requested a review from XiaobingSuper April 1, 2020 22:58
@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented Apr 1, 2020

💊 CircleCI build failures summary and remediations

As of commit a2e7aa8 (more details on the Dr. CI page):


None of the build failures appear to be your fault 💚


  • 2/2 broken upstream at merge base 8afa001 since Apr 07

    Please rebase on the viable/strict branch (expand for instructions)

    If your commit is newer than viable/strict, you can try basing on an older, stable commit:

    git fetch https://github.com/pytorch/pytorch viable/strict
    git rebase --onto FETCH_HEAD $(git merge-base origin/master HEAD)
    

    If your commit is older than viable/strict:

    git fetch https://github.com/pytorch/pytorch viable/strict
    git rebase FETCH_HEAD
    

    Check out the recency history of this "viable master" tracking branch.


🚧 2 upstream failures:

These were probably caused by upstream breakages:


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.

See how this bot performed.

This comment has been revised 24 times.

auto o2 = func(a_hi, b_hi); \
return cvtfp32_bf16(o1, o2); \
template<typename Op>
Vec256<BFloat16> inline bfloat16_binary_op_as_fp32(const Vec256<BFloat16>& a, const Vec256<BFloat16>& b, Op op) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@XiaobingSuper Do you think this function can also be used for implementing operators >, <, >=, and <=? Now #35117 should be waiting on these operators.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, there a PR #35092 doing this.

@XiaobingSuper XiaobingSuper requested a review from ngimel April 3, 2020 01:32
@zou3519 zou3519 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Apr 6, 2020
@xuhdev xuhdev requested a review from ezyang April 6, 2020 22:10
…16> bin operators"

Also, bitwise operators can operate on the underlying __m256i
representation directly instead of making expensive conversions to
float16.

[ghstack-poisoned]
…16> bin operators"

Also, bitwise operators can operate on the underlying __m256i
representation directly instead of making expensive conversions to
float16.

[ghstack-poisoned]
xuhdev added 2 commits April 6, 2020 19:25
…16> bin operators"

Also, bitwise operators can operate on the underlying __m256i
representation directly instead of making expensive conversions to
float16.

[ghstack-poisoned]
…16> bin operators"

Also, bitwise operators can operate on the underlying __m256i
representation directly instead of making expensive conversions to
float16.

[ghstack-poisoned]
}

Vec256<BFloat16> inline operator&(const Vec256<BFloat16>& a, const Vec256<BFloat16>& b) {
return _mm256_and_si256(a, b);
Copy link
Copy Markdown
Collaborator

@ngimel ngimel Apr 7, 2020

Choose a reason for hiding this comment

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

this is instruction for signed integers, not for floats? It used to be _mm256_and_ps, which is indeed instruction for floats. Ah, nm, I see what you are doing.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes. The point is that it is not necessary to convert to float in this case, because bitwise operators have the same effects. There are two different instructions for integers and float because they can be directly applied to different data types (__m256i and __m256).

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@ngimel merged this pull request in 0bc17dd.

@xuhdev xuhdev deleted the gh/xuhdev/69/head branch April 9, 2020 21:03
ashishfarmer pushed a commit to ashishfarmer/pytorch that referenced this pull request Apr 13, 2020
…rators (pytorch#35844)

Summary:
Pull Request resolved: pytorch#35844

Also, bitwise operators can operate on the underlying __m256i
representation directly instead of making expensive conversions to
float16.

Test Plan: Imported from OSS

Differential Revision: D20927639

Pulled By: ngimel

fbshipit-source-id: 148c503df090580c8504f0df8d6ed2648d614120
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
…rators (pytorch#35844)

Summary:
Pull Request resolved: pytorch#35844

Also, bitwise operators can operate on the underlying __m256i
representation directly instead of making expensive conversions to
float16.

Test Plan: Imported from OSS

Differential Revision: D20927639

Pulled By: ngimel

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

Labels

Merged open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants