[PATCH] D156087: [MLIR] Add stage to side effect

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

> I believe that when we say “write full” to a particular resource, this write is certain to occur. Therefore, any writes to this resource before that point can be killed by this operation. If the write is only possible, and we shouldn't explicitly mark it as writing all resources, do you think this approach is feasible?

It might be ok, I am not sure it is conservative in every situation? Assuming it is, it should likely be the default, right now your MemAlloc default to full.



================
Comment at: mlir/include/mlir/Interfaces/SideEffectInterfaces.td:38
+class MemoryEffect<string effectName, Resource resource, int stage,
+                   bit fullRegion>
+  : SideEffect<MemoryEffectsOpInterface, effectName, resource, stage,
----------------
cxy-1993 wrote:
> mehdi_amini wrote:
> > Do we have enums in TableGen? (I don't remember).
> > 
> > I rather read "Full" and "Partial" than true/false if possible.
> I can not found any enum in TableGen except EnumAttr...
I think it should look like this in TableGen:

```
class EffectRange <bits<1> val> {
  bits<1> Value = val;
}

def FullEffect : EffectStage<1>;
def PartialEffect : EffectStage<0>;

class SideEffect<EffectOpInterfaceBase interface, string effectName,
                 Resource resourceReference, int effectStage, EffectRange range> {
   ...

  // Does this side effect act on every single value of resource.
  EffectRange effectOnFullRegion = range;
}
```

But it may be possible to write also here:
```
  // Does this side effect act on every single value of resource.
  bit effectOnFullRegion = range.Value;
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156087/new/

https://reviews.llvm.org/D156087