-
Notifications
You must be signed in to change notification settings - Fork 212
[WIP] Add bitwise operators and propagate them from parser -> halide -> isl #380
base: add-operators
Are you sure you want to change the base?
Conversation
@skimo-openhub , @ftynse , how do the operators propagate to isl? I see files like |
Note that the Halide simplifier will convert all the simpler bit operations to multiplication (x << 5), division (x >> 5), and modulo (x & 0x3f), so the only bit operations that reach isl will be the nastier ones. That said, bit operations do have well-defined bounds that we may be able to exploit (e.g. for unsigned ints (x & y) <= x) |
thanks @abadams . I have two questions for @skimo-openhub and @ftynse:
I'll try to add simple unit tests for these to catch errors on propagation to isl |
(quick answer, I'll give more details when not on the phone) We need to differentiate two cases for bitwise operations, similarly to modulo. If they appear in tensor subscripts, those are not affine and should be overapproximated. Outer operations should be handled as function calls essentially. |
274aa28
to
6b0382a
Compare
random thought: I would add checks in |
thanks. I will wait to get more details to understand better the cases to distinguish. In the meantime, propagation from halide to isl doesn't work by default. here is the codegen for one example:
Note the |
isl must be agnostic to these operations. Essentially, the level we are interested about in isl is below those. At that level, there is no difference between In this particular case, the simplest solution is to inject |
Now, if operations appear in subscripts, like Bitiwise operations in general are not affine, so you would need that over-approximation. It should be rather straightforward to implement in |
61c762d
to
3abd114
Compare
thanks @ftynse, the bitwise operators work now for the simple test case added to check correctness. can you provide what more test cases should I test this on (especially over-approximation, affine/non-affine scenarios)? I believe that would help me understand more how to address those cases by digging more into the codebase :) thanks a lot for your guidance. |
0960f03
to
022c62a
Compare
the 'if' condition is checked and 'return' happens if the condition is met. Using 'else' is not needed
proper variable naming and whitelining between functions
change makeIslAffBoundsFromExpr(..., e, ...) to makeIslAffBoundsFromExpr(..., op->a, ...) to avoid infinite recursion leading to segfault
support % operator: propagate it from parser to halide to isl and add unit tests
dead function and not used anywhere so cleaning it up
3abd114
to
892d795
Compare
add me back when there is any progress |
8af6370
to
ab72ede
Compare
ab72ede
to
b36f466
Compare
Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired. Before we can review or merge your code, we need you to email [email protected] with your details so we can update your status. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
1 similar comment
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
trying to add bitwise operators. so far they are parsed correctly in their precedence order.
tc2halide
conversion is also done.I am not sure about how to propagate these through to isl yet.
NOTE: I stacked this PR on add-operators branch to avoid merge conflicts.
closes #131