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

segfault on rfactoring an RVar produced by fusing a Var with an RVar #7854

Open
abadams opened this issue Sep 19, 2023 · 3 comments · May be fixed by #8268
Open

segfault on rfactoring an RVar produced by fusing a Var with an RVar #7854

abadams opened this issue Sep 19, 2023 · 3 comments · May be fixed by #8268
Assignees
Labels

Comments

@abadams
Copy link
Member

abadams commented Sep 19, 2023

This crashes:

#include "Halide.h"

using namespace Halide;

int main(int argc, char **argv) {
    Func f;
    RDom r(0, 100);
    Var x, y;
    f(x, y) = 0;
    f(x, y) += r;

    RVar yr;
    Var z;
    f.update().fuse(y, r, yr).rfactor(yr, z);

    return 0;
}

I believe the issue is that the rfactor code has to handle splits, but doesn't correctly handle fusing a Var with an RVar to produce an RVar. Also it doesn't do enough validation. This should be an internal assert failure rather than a crash.

@abadams abadams added the bug label Sep 19, 2023
@abadams abadams self-assigned this Sep 27, 2023
@abadams
Copy link
Member Author

abadams commented Jun 5, 2024

The fix is to forbid rfactoring over an RVar if that RVar has had a pure var fused into it

@alexreinking alexreinking self-assigned this Jun 5, 2024
@alexreinking
Copy link
Member

Why doesn't this just set yr=r and z=y?

@abadams
Copy link
Member Author

abadams commented Jun 5, 2024

That sounds more like a split than an rfactor. rfactor would be replacing the fusion of y and r with a single new pure Var z in an intermediate Func, and moving the reduction over yr (still fused) to the wrapper Func.

The problem with that is you need bounds for yr to do that transformation, which don't exist yet where rfactor is implemented because the bounds of yr depend on the bounds of y which are yet to be inferred, and even when inferred it places a nasty constraint on what the bounds of y can be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants