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

Call::prefetch's return type indicates the type being prefetched, which does not match the implementation #4211

Closed
dsharletg opened this issue Sep 5, 2019 · 1 comment

Comments

@dsharletg
Copy link
Contributor

This change of #3667: https://github.com/halide/Halide/pull/3667/files#diff-a8d8ceb2b87d97fbb8fe6eea48d2178f broke prefetching. Apparently, the return type of Call::prefetch indicates the type of prefetching to perform in some places, e.g.

<< "((" << print_type(op->type) << " *)" << print_name(base->name)

But not others, e.g.:

args.push_back(codegen_buffer_pointer(codegen(op->args[0]), op->type, op->args[1]));

It seems reasonable-ish for prefetch to be defined this way (it looks kind of like a load), but it is difficult to implement. If we want to keep this behavior, then I think we need to make a special case for this function in CodeGen_LLVM and CodeGen_Hexagon.

For now, I am going to send a PR to fix the breakage by adding an exception for Call::prefetch to the new assert, but I'm going to leave this issue open because this seems like it needs improvement.

steven-johnson added a commit that referenced this issue Jun 8, 2023
As noted in the issue above, the return type of this Call was used weirdly and wrongly: it wasn't the return type of the intrinsic (which is always int32, and always ignored), but rather, the type of the elements to prefetch. This didn't seem to be causing any obvious errors, but it meant we needed some weird special cases in the code. We now insert another element in the args to carry the type needed (with a value of constant zero).
abadams added a commit that referenced this issue Jun 12, 2023
Call::Prefetch evaluates to a currently-unspecified value of the
prefetched type. Let's just make it zero.
steven-johnson added a commit that referenced this issue Jun 17, 2023
* Alternative fix for #4211

Call::Prefetch evaluates to a currently-unspecified value of the
prefetched type. Let's just make it zero.

* Fix prefetch_2d

* Fix CodeGen_C

* Fix CodeGen_C

* trigger buildbots

---------

Co-authored-by: Steven Johnson <srj@google.com>
@steven-johnson
Copy link
Contributor

Fixed

ardier pushed a commit to ardier/Halide-mutation that referenced this issue Mar 3, 2024
* Alternative fix for halide#4211

Call::Prefetch evaluates to a currently-unspecified value of the
prefetched type. Let's just make it zero.

* Fix prefetch_2d

* Fix CodeGen_C

* Fix CodeGen_C

* trigger buildbots

---------

Co-authored-by: Steven Johnson <srj@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants