core: Implement EffectKind #2793
Conversation
…t ops with reads and writes, and do a minor swap in trivially_dead; to prepare the more subtle implementations.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2793 +/- ##
=======================================
Coverage 89.83% 89.83%
=======================================
Files 396 396
Lines 49139 49233 +94
Branches 7533 7546 +13
=======================================
+ Hits 44145 44230 +85
- Misses 3804 3813 +9
Partials 1190 1190 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
There was a problem hiding this comment.
PR and code LGTM and matches MLIR with the more limited feature set (any reason this is still a draft acutally? I'd stamp otherwise).
One thing worth considering at your discretion:
One annoyance I've always had with this interface in MLIR is that if an Op implemented the trait/interface, it was impossible for the op to dynamically (based on attributes e.g.) be able to say "I am side effect free" or "I affect the universe" (the same as an op not implementing the interface basically). This is because both in this PR and in MLIR, returning an empty set means "no side effect".
If it instead returned a set | None (or similar) then we could encode None to mean the same as a missing trait implementation (effects the universe).
A common ues case for this would be a call op e.g. The call op may be annotated with a pure attribute and in that case you want it to not have any side effects. If it doesn't, you'll want it to affect the universe
Thought I'd mention this now so that if you do want it, that you don't have to go through the refacotring churn twice.
|
Interesting point, thanks!! I'll give it some thought and would love to discuss it, as I'm lass experienced on that side of things. My first thought is to ask if that option isn't still limited. It still means arbitrary but finite effects of some kind, or absolutely everything, right? What about reading the universe while writing to a defined part? Genuinely wondering as I'm not sure it would either represent something practical or enable more powerful anything 🤔 |
It generally means absolutely everything yes. The most conservative answer.
Good question. This is currently unimplemented in MLIR as well as there is no special value for the universe right now. I've seen this come up from time to time in discussions but haven't seen anyone actually try to implement it. MLIR does have the concept of |
Right, I guess the best end-game answer is building an aliasing framework for resources indeed, having a Universe defined as aliasing everything. Thanks a lot for the discussion though, didn't cross my mind yet! |
|
I hear your point; to clarify mine, by sugar, I meant my own proposal of helper base classes, not the composability, which indeed is a bit more core. I'll add composability; unless I realize an awful consequence or what not On the sugar : I think the vast majority of implementations will be "have effect(s) a if b(op)", so a cute wrapper parametrized by an effect and predicate would help a lot I think; especially with composability included! |
Ah yes, this makes much more sense!
Let's hope it's all fine! 🤞
Agree 100%. I guess I could see two traits for different kinds of efffects (e.g. Looking forward to us being at that stage! |
…- trade performance for readability and less points of failure.
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
…veMemoryEffect in terms of MemoryEffect again. rename to get_effets.
Implement EffectKind, sepcifying READ, WRITE, ALLOC or FREE side-effects.
Use it to enable our MemoryEffect to return this more precise information, and leverage it in DCE (an unused read-only effect is safe to erase) and CSE (A duplicate read-only is safe to replace with an earlier one if we can ensure there was no write in-between)
MLIR further enables to express side-effects tied to specific resources and values, but this a little step forward.