Bounds and alignment analysis through bitwise ops #8574
Conversation
converting it to and from the existing ExprInfo as necessary
| result.mask |= (uint64_t)(-1) << (type.bits() - 1); | ||
| } else if (type.bits() < 64) { | ||
| // Narrow uints are zero-extended. | ||
| result.mask |= (uint64_t)(-1) << type.bits(); |
There was a problem hiding this comment.
Isn't this true regardless of the bounds?
| @@ -496,5 +496,114 @@ bool can_prove(Expr e, const Scope<Interval> &bounds) { | |||
| return is_const_one(e); | |||
| } | |||
|
|
|||
| Simplify::ExprInfo::BitsKnown Simplify::ExprInfo::to_bits_known(const Type &type) const { | |||
There was a problem hiding this comment.
A lot of this bitwise math is a little tricky to follow. Have you thrown this in an SMT solver? I think this should be verified
src/Simplify_Call.cpp
Outdated
| if (info && (op->type.is_int() || op->type.is_uint())) { | ||
| auto bits_known = a_info.to_bits_known(op->type) & b_info.to_bits_known(op->type); | ||
| info->from_bits_known(bits_known, op->type); | ||
| if (bits_known.mask == (uint64_t)-1) { |
There was a problem hiding this comment.
Add comment saying something like "This is a constant". Or make ExprInfo::BitsKnown have a helper function like "std::optional<uint64_t> as_constant()`.
There was a problem hiding this comment.
and same below with bitwise_or
|
|
||
| Expr unbroadcast = lift_elementwise_broadcasts(op->type, op->name, {a, b}, op->call_type); | ||
| if (unbroadcast.defined()) { | ||
| return mutate(unbroadcast, info); | ||
| } | ||
|
|
||
| if (info && (op->type.is_int() || op->type.is_uint())) { | ||
| auto bits_known = a_info.to_bits_known(op->type) ^ b_info.to_bits_known(op->type); | ||
| info->from_bits_known(bits_known, op->type); |
There was a problem hiding this comment.
same here, check for a constant?
src/Simplify_Internal.h
Outdated
| } | ||
|
|
||
| BitsKnown operator|(const BitsKnown &other) const { | ||
| uint64_t zeros = known_zeros() & other.known_zeros(); |
There was a problem hiding this comment.
Add a comment like in the & case
Some fuzzing with extra checks turned on revealed that the simplifier would often produce constants without the ExprInfo knowing they were constants. In a few places we were doing a janky remutation of a known constant just to set the info, but this wasn't being consistently done. This commit changes constant-folding in the simplifier and info handling in boolean ops to know that it's a constant value being returned. Also standardize the spelling of zeros
|
I delayed this until after the siggraph asia deadline, but now that it has passed, ptal |
test/correctness/bits_known.cpp
Outdated
| // op. Currently just known the second-lowest-bit is 2 but nothing else | ||
| // doesn't give us an alignment or bounds. |
There was a problem hiding this comment.
I am struggling to follow the grammar of this sentence
There was a problem hiding this comment.
Me too! I'll try to reconstruct what I meant.
I encountered a case in production where slow code was being generated because Halide didn't recognize
(x + 15) & ~15 - y & ~15as a multiple of 16, so it failed to vectorize an atomic because a GuardWithIf if statement got in the way. This was a new issue for Halide 19, because previously the GuardWithIf if statement was incorrectly being dropped by a now-fixed bug in rfactor.This PR adds support for constant bounds analysis and alignment analysis in the simplifier for bitwise ops. It does this by converting ExprInfo to a more LLVM-style BitsKnown structure temporarily.