Reenable warning about unscheduled update definitions#6602
Conversation
and fix associated issues in the tests and apps. This is an old warning that stopped triggering because it wasn't tested. We should either remove it, fix the trigger conditions, or perhaps make it an error. This PR fixes the trigger conditions and fixes all instances of the warning in our tests and apps. The warning triggers if you schedule some but not all of the update definitions of a Func. It's to protect against the common error of only scheduling the pure definition of something like a summation. The warning can be suppressed by inserting a call to func.update(idx).
steven-johnson
left a comment
There was a problem hiding this comment.
Lots of broken stuff, removing approval
|
Looks like an llvm master breakage, and an uncovered bug in atomics. I'll investigate. |
Could bit be related to https://reviews.llvm.org/D115955 ? (It was reverted last night, I'm working on getting a repro case for them) |
…scheduled_stage_warning
Seeing |
|
So where do we stand with this? Do we want to land it as-is and hope to do a proper linter tool in the future? Or what? |
|
Up to Andrew, I guess. My suggestion was just a suggestion. |
|
This looks great! Thanks for indulging me, @abadams 🙂 |
|
The Windows failure appears to be a flake, although possibly a real bug (opened issue as #6621). OK to land. |
and fix associated issues in the tests and apps.
This is an old warning that stopped triggering because it wasn't tested.
We should either remove it, fix the trigger conditions, or perhaps make
it an error. This PR fixes the trigger conditions and fixes all
instances of the warning in our tests and apps.
The warning triggers if you schedule some but not all of the update
definitions of a Func. It's to protect against the common error of only
scheduling the pure definition of something like a summation. The
warning can be suppressed by inserting a call to func.update(idx).