Skip to content

Redirect bitwise ops to logical ops in case the arguments are bool. #8597

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
Mar 28, 2025

Conversation

mcourteaux
Copy link
Contributor

@mcourteaux mcourteaux commented Mar 17, 2025

This fixes an explicit type error on the WebGPU backend, where WGSL is not allowing bool ^ bool. Fixes #8596.

@abadams abadams added the dev_meeting Topic to be discussed at the next dev meeting label Mar 20, 2025
@abadams
Copy link
Member

abadams commented Mar 20, 2025

Failures unrelated. This makes sense, but this seems bordering on a language semantics change, and raises the oddity of having a bitwise_and intrinsic that's distinct from the And node even though they're identical for bools. I want to make sure we discuss this briefly in a dev meeting just to get a sanity check from others.

@mcourteaux
Copy link
Contributor Author

mcourteaux commented Mar 20, 2025

I think the main argument is that Halide doesn't have a boolean type, and instead relies on uint1. So really we cannot make the distinction between a 1-bit int, and a boolean. So either we remove all logical operators (and say everything is an int, and therefore there are no logical operators to be defined on ints). Or we make the bitwise operators on uint1 redirect to logical operators.

Also note that the Python bindings do not even expose the logical AND operator (because python does not allow to override it), and instead relies on the users to use bitwise-AND & and bitwise-OR |.

@alexreinking
Copy link
Member

alexreinking commented Mar 26, 2025

I'm actually running into this exact issue with PyTorch now. Some code gets generated that resolves to a bitwise or in the condition of a select, which should be possible to rfactor, but isn't recognized in the patterns.

There really should be a halide.Or and halide.And function in the Python bindings.

@mcourteaux
Copy link
Contributor Author

I was more conservative on changing the Python bindings API, as probably there are quite some people using that. Initially, I had added asserts that make sure you don't try to bool & bool etc, but that triggered those asserts in Python. So I thought that simply correcting those calls to their equivalent logical operators made the most sense without breaking anyone's code.

@abadams
Copy link
Member

abadams commented Mar 28, 2025

Dev meeting decision: We want to normalize bitwise_and and the like to And for bools, because the compiler reasons about And more aggressively than bitwise ops, and they're semantically identical for bools (because we don't short-circuit). The question is when? We decided that it's reasonable to do it as early as possible, eagerly, in IROperator.cpp, so that it's already done for early-stage compilation things like rfactor. There's some precedent for that, e.g. absd(x, y) on floats actually returns abs(x - y).

@mcourteaux mcourteaux merged commit f182392 into halide:main Mar 28, 2025
10 of 11 checks passed
@alexreinking alexreinking added the release_notes For changes that may warrant a note in README for official releases. label Mar 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev_meeting Topic to be discussed at the next dev meeting release_notes For changes that may warrant a note in README for official releases.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Binary bitwise operators ^, &, | should be invalid on Bools.
3 participants