Skip to content
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

RDoms extending past the edge of an image trigger assert, even though .where clause prevents bad memory access #2373

Closed
steven-johnson opened this issue Sep 22, 2017 · 3 comments

Comments

@steven-johnson
Copy link
Contributor

commented Sep 22, 2017

From an internal source:


Consider the following code:

Var x, y, z;
RDom r(0, 32);
r.where(x >= r);

Func volume;
volume(x, y, z) = Halide::undef<uint32_t>();
volume(x, y, r) = input(x - r, y);

This code fails to generate with a complaint about out of bounds access. The assert is part of the big pile of asserts in the preamble of the function, and can be turned off by compiling with target feature no_asserts, so it is not a blocker; but it would be nice if I could retain the safety of the the access checks in this scenario.

@psuriana

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2017

The current bound inference doesn't consider the correlation between x and r from the predicate. So you'll end up with [xmin - rmax, xmax - rmin] for the interval of x - r, causing the out of bound assertion. To be able to generate tighter bound check, we'll need a full polyhedral analysis which would be pretty expensive. The easiest way to deal with this problem would probably to just clamp the access to input.

@khourig

This comment has been minimized.

Copy link

commented Sep 22, 2017

I ran into this same issue pretty soon after I started working with Halide. I was calculating an index that I knew was going to be within the bounds of a buffer and then using it to look up into that buffer. However, I was forced to clamp the value when using it as a lookup index. At the time, I lobbied for a f.ranged(lo, hi) intrinsic that would let me pass this knowledge to Halide explicitly. The values can be checked against the bounds with asserts and this can be shut off for a final release build. Optimally, the information would be attached to the Func object and the value asserts done at storage time.

@khourig

This comment has been minimized.

Copy link

commented Sep 22, 2017

Now that I think about it, it would be an Expr modifier like clamp. I still think that once this reaches the Func level, the value bounds need to be stored there so consumers of the Func can know the explicit value bounds.

@psuriana psuriana closed this Jan 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.