search

halide / Halide

a language for fast, portable data-parallel computation
https://halide-lang.org
Other
5.86k stars 1.07k forks source link

Refactor ConstantInterval #8179

Closed abadams closed 5 months ago

abadams commented 6 months ago

This is the bulk of #8155, pulled out for separate review.

This PR pulls the ConstantInterval class out of Interval.h, and the arithmetic helpers for it out of Monotonic.cpp. It then refactors Monotonic.cpp to use it, including using a new bounds inference routine to use instead of find_constant_bounds. It adds fuzz tests for both the new bounds inference routine and the interval arithmetic itself.

The fuzz test shows a lot of errors due to the lossless_cast casting bugs revealed in #8155, so those are simply reported for now, though I did fix the most egregious bug in lossless_cast to reduce the noise. The fuzz test also found another error to fix in FindIntrinsics.cpp

After this, the next step is to refactor the simplifier to use this ConstantInterval instead of rolling its own thing, and then actually fixing the lossless_cast bug, which has trickle-on effects in instruction selection.

abadams commented 6 months ago

@rootjalex I think you would be ideal to review this (Not urgently - once you're out from under your current crunch)

rootjalex commented 6 months ago

Will take a look next week!

abadams commented 5 months ago

I believe I have addressed review comments, PTAAL (mostly for operator<<)

abadams commented 5 months ago

Well now there's a fuzz test failure, so something's probably not right with the new operator<<. Will investigate.

abadams commented 5 months ago

The fuzzer failure was just lossless_negate being wrong, which is to be fixed in #8155