Skip to content

[flang] Check for overflows in RESHAPE folding #68342

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

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

luporl
Copy link
Contributor

@luporl luporl commented Oct 5, 2023

TotalElementCount() was modified to return std::optional<uint64_t>,
where std::nullopt means overflow occurred. Besides the additional
check in RESHAPE folding, all callers of TotalElementCount() were
changed, to also check for overflows.

@luporl
Copy link
Contributor Author

luporl commented Oct 5, 2023

This PR fixes part of the problem of #63770.

Copy link
Contributor

@klausler klausler left a comment

Choose a reason for hiding this comment

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

A better change would be to alter the API to TotalElementCount() so that it returns std::optional<uint64_t> with a value present only if no overflow occurs. This would force all call sites to check for and deal with overflow.

@luporl
Copy link
Contributor Author

luporl commented Oct 17, 2023

Thanks for the review. I've modified TotalElementCount() to return std::optional<uint64_t> and changed all of its callers to check for overflows.

TotalElementCount() was modified to return std::optional<uint64_t>,
where std::nullopt means overflow occurred. Besides the additional
check in RESHAPE folding, all callers of TotalElementCount() were
changed, to also check for overflows.
@luporl luporl force-pushed the luporl-reshape-ov branch from c8449bd to 86cd08d Compare October 23, 2023 20:29
@luporl
Copy link
Contributor Author

luporl commented Oct 30, 2023

Ping

Copy link
Contributor

@psteinfeld psteinfeld left a comment

Choose a reason for hiding this comment

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

All builds and tests correctly. But since @klausler has already commented, I'll leave it to him to approve.

@luporl luporl merged commit 7358c26 into llvm:main Oct 30, 2023
@luporl luporl deleted the luporl-reshape-ov branch October 30, 2023 17:25
kkwli added a commit that referenced this pull request Nov 6, 2023
PR #68342 causes build breakage on MacOS due to uint64_t being defined
as unsigned long long instead of unsigned long. It leads to type
mismatch in the arguments for std::min.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants