Skip to content

Define cast<i32>(u32) overflow behavior #7769

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.

Sign up for GitHub

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

Jump to bottom
Merged
abadams merged 1 commit into from
Aug 17, 2023
Merged

Define cast<i32>(u32) overflow behavior #7769

abadams merged 1 commit into from
Aug 17, 2023

Conversation

@rootjalex
Copy link
Member

@rootjalex rootjalex commented Aug 16, 2023

The simplifier normalizes reinterpret<i32>(u32) (which has defined behavior) to cast<i32>(u32), which has undefined behavior. Currently, the following code produces inconsistent output:

{
    Expr x = UIntImm::make(UInt(32), 0xFFFFFFF0u);
    Expr e = i32(x + 1);
    e = simplify(e);
    std::cout << e << std::endl;

    e = reinterpret(Int(32), x + 1);
    e = simplify(e);
    std::cout << e << std::endl;
}
{
    Expr x = Variable::make(UInt(32), "x");
    Expr e = i32(x + 1);
    e = simplify(e);
    e = substitute("x", UIntImm::make(UInt(32), 0xFFFFFFF0), e);
    e = simplify(e);
    std::cout << e << std::endl;

    e = reinterpret(Int(32), x + 1);
    e = simplify(e);
    e = substitute("x", UIntImm::make(UInt(32), 0xFFFFFFF0), e);
    e = simplify(e);
    std::cout << e << std::endl;
}

Output:

signed_integer_overflow(0)
-15
signed_integer_overflow(1)
signed_integer_overflow(2)

@abadams and I agreed to simply define cast<i32>(u32) to have the same 2's complement wrapping behavior as reinterpret<i32>(u32). I think the only place this behavior is encoded is in the simplifier (LLVM codegen produces a no-op for either case because LLVM types don't have signedness).

Now the output of the above code is:

-15
-15
-15
-15

@rootjalex rootjalex requested a review from abadams August 16, 2023 21:41
@steven-johnson
Copy link
Contributor

steven-johnson commented Aug 16, 2023

SGTM

@abadams abadams merged commit f2f2af2 into main Aug 17, 2023
@abadams abadams mentioned this pull request Aug 24, 2023
Open
@steven-johnson
Copy link
Contributor

steven-johnson commented Aug 28, 2023

No idea if these are related to this change or not, but:

#7810
#7811

(Just reported by Google-internal fuzz testing after our most recent merge from GitHub)

@abadams abadams added the release_notes For changes that may warrant a note in README for official releases. label Aug 28, 2023
@BrewTestBot BrewTestBot mentioned this pull request Feb 2, 2024
Closed
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
@rootjalex @ardier
Define cast<i32>(u32) overflow behavior (halide#7769)
560fe47 
uint32 -> int32 casting should not produce SIO
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abadams abadams abadams approved these changes

Assignees

No one assigned

Labels

release_notes For changes that may warrant a note in README for official releases.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rootjalex @steven-johnson @abadams