-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add fixes to overflow analysis in bounds inference #5618
Conversation
…halide/Halide into rootjalex/bounds_overflow_fixes
@abadams Fixed your comments and also added |
…lex/bounds_overflow_fixes
…halide/Halide into rootjalex/bounds_overflow_fixes
I'd argue that, as a rule of thumb, all TODOs that are checked in to the codebase should generally have a tracking issue (referenced in the comment). Not always, but usually. |
…halide/Halide into rootjalex/bounds_overflow_fixes
Sorry for the delay @steven-johnson . I created #5682 for this problem, and updated the TODOs in 6fa0a56 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits.
@abadams is this ready to land? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with two comment nits
…lex/bounds_overflow_fixes
…lex/bounds_overflow_fixes
…lex/bounds_overflow_fixes
Add fixes (and tests) to bounds inference for the following cases:
UInt(32)
addition and multiplication overflow was not properly detected.type.min()
by-1
forInt(16)
andInt(8)
was not detected as overflow.lanes
parameter would produce incorrect bounds (e.g. a ramp of bools)