[mlir] Updated SideEffect interface definitions to support tablegen Resource objects.

[dfki-mako]

Summary

The SideEffect interface definitions currently use string expressions to reference custom resource objects. This CL introduces Resource objects in tablegen definitions to simplify linking of resource reference to resource objects.

[herhut]

I like the extra bit of structure, especially the DefaultResource is more readable than "". @rriddle created this, so wait for his approval.

[dfki-mako]

All right; let's wait for @rriddle's comments

[mehdi_amini]

Nice improvement!

In D78917#2009878, @dfki-mako wrote:

All right; let's wait for @rriddle's comments

Did you get it?

[dfki-mako]

@mehdi_amini Thanks a lot for your feedback. Unfortunately, I haven't received some comments from him.

[mehdi_amini]

In D78917#2041123, @dfki-mako wrote:

@mehdi_amini Thanks a lot for your feedback. Unfortunately, I haven't received some comments from him.

From what I see above, this revision was not approved without these though? Why did this land?

[herhut]

In D78917#2042440, @mehdi_amini wrote:

In D78917#2041123, @dfki-mako wrote:

@mehdi_amini Thanks a lot for your feedback. Unfortunately, I haven't received some comments from him.

From what I see above, this revision was not approved without these though? Why did this land?

We talked about this offline and decided to move forward with this change. We did not feel this was controversial and also easy to roll back in case there are comments later.

[rriddle]

Resources in the current form are only temporary anyways, so i don't see a problem with this.

mlir/include/mlir/Interfaces/SideEffects.td

 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_INTERFACES_SIDEEFFECTS
 #define MLIR_INTERFACES_SIDEEFFECTS
 
 include "mlir/IR/OpBase.td"
 
 //===----------------------------------------------------------------------===//
+// Resource Bindings
+//===----------------------------------------------------------------------===//
+
+// A generic resource that can be attachted to a general base side effect.
+class Resource<string resourceName> {
+  /// The resource that the associated effect is being applied to.
+  string name = resourceName;
+}
+
+// An intrinsic resource that lives in the ::mlir::SideEffects namespace.
+class IntrinsicResource<string resourceName> :
+  Resource<!strconcat("::mlir::SideEffects::", resourceName)> {
+}
+
+// A link to the DefaultResource class.
+def DefaultResource : IntrinsicResource<"DefaultResource">;
+
+//===----------------------------------------------------------------------===//
 // EffectOpInterface
 //===----------------------------------------------------------------------===//
 
 // A base interface used to query information about the side effects applied to
 // an operation. This template class takes the name of the derived interface
 // class, as well as the name of the base effect class.
 class EffectOpInterfaceBase<string name, string baseEffect>
     : OpInterface<name> {
class EffectOpInterfaceBase<string name, string baseEffect>
 
   // The base effect name of this interface.
   string baseEffectName = baseEffect;
 }
 
 // This class is the general base side effect class. This is used by derived
 // effect interfaces to define their effects.
 class SideEffect<EffectOpInterfaceBase interface, string effectName,
-                 string resourceName> : OpVariableDecorator {
+                 Resource resourceReference> : OpVariableDecorator {
   /// The name of the base effects class.
   string baseEffectName = interface.baseEffectName;
 
   /// The parent interface that the effect belongs to.
   string interfaceTrait = interface.trait;
 
   /// The derived effect that is being applied.
   string effect = effectName;
 
   /// The resource that the effect is being applied to.
-  string resource = resourceName;
+  string resource = resourceReference.name;
 }
 
 // This class is the base used for specifying effects applied to an operation.
 class SideEffectsTraitBase<EffectOpInterfaceBase parentInterface,
                            list<SideEffect> staticEffects>
     : OpInterfaceTrait<""> {
   /// The name of the interface trait to use.
   let trait = parentInterface.trait;
: EffectOpInterfaceBase<"MemoryEffectOpInterface",
                             "MemoryEffects::Effect"> {
   let description = [{
     An interface used to query information about the memory effects applied by
     an operation.
   }];
 }
 
 // The base class for defining specific memory effects.
-class MemoryEffect<string effectName, string resourceName>
-  : SideEffect<MemoryEffectsOpInterface, effectName, resourceName>;
+class MemoryEffect<string effectName, Resource resource>
+  : SideEffect<MemoryEffectsOpInterface, effectName, resource>;
 
 // This class represents the trait for memory effects that may be placed on
 // operations.
 class MemoryEffects<list<MemoryEffect> effects = []>
   : SideEffectsTraitBase<MemoryEffectsOpInterface, effects>;
 
 //===----------------------------------------------------------------------===//
 // Effects
 
 // The following effect indicates that the operation allocates from some
 // resource. An 'allocate' effect implies only allocation of the resource, and
 // not any visible mutation or dereference.
-class MemAlloc<string resourceName>
-  : MemoryEffect<"MemoryEffects::Allocate", resourceName>;
-def MemAlloc : MemAlloc<"">;
+class MemAlloc<Resource resource>
+  : MemoryEffect<"MemoryEffects::Allocate", resource>;
+def MemAlloc : MemAlloc<DefaultResource>;
 
 // The following effect indicates that the operation frees some resource that
 // has been allocated. A 'free' effect implies only de-allocation of the
 // resource, and not any visible allocation, mutation or dereference.
-class MemFree<string resourceName>
-  : MemoryEffect<"MemoryEffects::Free", resourceName>;
-def MemFree : MemFree<"">;
+class MemFree<Resource resource>
+  : MemoryEffect<"MemoryEffects::Free", resource>;
+def MemFree : MemFree<DefaultResource>;
 
 // The following effect indicates that the operation reads from some
 // resource. A 'read' effect implies only dereferencing of the resource, and
 // not any visible mutation.
-class MemRead<string resourceName>
-  : MemoryEffect<"MemoryEffects::Read", resourceName>;
-def MemRead : MemRead<"">;
+class MemRead<Resource resource>
+  : MemoryEffect<"MemoryEffects::Read", resource>;
+def MemRead : MemRead<DefaultResource>;
 
 // The following effect indicates that the operation writes to some
 // resource. A 'write' effect implies only mutating a resource, and not any
 // visible dereference or read.
-class MemWrite<string resourceName>
-  : MemoryEffect<"MemoryEffects::Write", resourceName>;
-def MemWrite : MemWrite<"">;
+class MemWrite<Resource resource>
+  : MemoryEffect<"MemoryEffects::Write", resource>;
+def MemWrite : MemWrite<DefaultResource>;
 
 //===----------------------------------------------------------------------===//
 // Effect Traits
 //===----------------------------------------------------------------------===//
 
 // Op has no side effect.
 def NoSideEffect : MemoryEffects<[]>;
 // Op has recursively computed side effects.
 def RecursiveSideEffects : NativeOpTrait<"HasRecursiveSideEffects">;
 
 #endif // MLIR_INTERFACES_SIDEEFFECTS

mlir/lib/TableGen/SideEffects.cpp

StringRef SideEffect::getBaseEffectName() const {
   return def->getValueAsString("baseEffectName");
 }
 
 StringRef SideEffect::getInterfaceTrait() const {
   return def->getValueAsString("interfaceTrait");
 }
 
 StringRef SideEffect::getResource() const {
-  auto value = def->getValueAsString("resource");
-  return value.empty() ? "::mlir::SideEffects::DefaultResource" : value;
+  return def->getValueAsString("resource");
 }
 
 bool SideEffect::classof(const Operator::VariableDecorator *var) {
   return var->getDef().isSubClassOf("SideEffect");
 }
 
 //===----------------------------------------------------------------------===//
 // SideEffectsTrait

mlir/test/mlir-tblgen/op-side-effects.td

 // RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
 
 include "mlir/Interfaces/SideEffects.td"
 
 def TEST_Dialect : Dialect {
   let name = "test";
 }
 class TEST_Op<string mnemonic, list<OpTrait> traits = []> :
     Op<TEST_Dialect, mnemonic, traits>;
 
+def CustomResource : Resource<"CustomResource">;
+
 def SideEffectOpA : TEST_Op<"side_effect_op_a"> {
   let arguments = (ins Arg<Variadic<AnyMemRef>, "", [MemRead]>);
-  let results = (outs Res<AnyMemRef, "", [MemAlloc<"CustomResource">]>);
+  let results = (outs Res<AnyMemRef, "", [MemAlloc<CustomResource>]>);
 }
 
 def SideEffectOpB : TEST_Op<"side_effect_op_b",
-    [MemoryEffects<[MemWrite<"CustomResource">]>]>;
+    [MemoryEffects<[MemWrite<CustomResource>]>]>;
 
 // CHECK: void SideEffectOpA::getEffects
 // CHECK:   for (Value value : getODSOperands(0))
 // CHECK:     effects.emplace_back(MemoryEffects::Read::get(), value, ::mlir::SideEffects::DefaultResource::get());
 // CHECK:   for (Value value : getODSResults(0))
 // CHECK:     effects.emplace_back(MemoryEffects::Allocate::get(), value, CustomResource::get());
 
 // CHECK: void SideEffectOpB::getEffects
 // CHECK:   effects.emplace_back(MemoryEffects::Write::get(), CustomResource::get());