Skip to content

Conversation

@swolchok
Copy link
Contributor

This should fix the problem where attempts to test bool are often wonky in OSS and fail UBSAN internally; it is undefined behavior to store a value other than 0 or 1 for type bool.

[ghstack-poisoned]
@swolchok
Copy link
Contributor Author

swolchok commented Jan 22, 2025

@pytorch-bot
Copy link

pytorch-bot bot commented Jan 22, 2025

🔗 Helpful Links

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

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

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 Jan 22, 2025
dynamism) {}
dynamism) {
// The only valid values for bool are 0 and 1; coerce!
if constexpr (std::is_same_v<true_ctype, bool>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems in ifdef aten section this transformation is unconditional while here it is applied only if it is bool. I dont fully understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it doesn't build with bits16 and we don't have bit_cast, otherwise I would make it unconditional here and use bit_cast.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am guessing bit_cast not being usable is related to it available only in c++20.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes; we will have access to c10::bit_cast once I get back to code sharing but currently we don't have it.

@swolchok swolchok merged commit d08b9e6 into main Jan 23, 2025
43 of 44 checks passed
@swolchok swolchok deleted the gh/swolchok/178/head branch January 23, 2025 01:00
YIWENX14 pushed a commit that referenced this pull request Jan 28, 2025
* Coerce to true_ctype in tensor_factory (#7856)

This should fix the problem where attempts to test bool are often wonky in OSS and fail UBSAN internally; it is undefined behavior to store a value other than 0 or 1 for type bool.

* Support Half/BFloat16 in prod operator (#7857)

Partial fix for #7748.
zonglinpeng pushed a commit to zonglinpeng/executorch that referenced this pull request Jan 30, 2025
* Coerce to true_ctype in tensor_factory (pytorch#7856)

This should fix the problem where attempts to test bool are often wonky in OSS and fail UBSAN internally; it is undefined behavior to store a value other than 0 or 1 for type bool.

* Support Half/BFloat16 in prod operator (pytorch#7857)

Partial fix for pytorch#7748.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. topic: not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants