[Kanagawa] Remove %this (#8097)

`%this` and the `ThisOp` isn't being used in practice, and is now pure overhead. This PR removes the `ThisOp` operation and any associated capabilities. Most notably, this includes referencing ports of the current scope, within the current scope, through `get_port` on the `ThisOp`. This, again, is a redundant pattern, that anyways were being canonicalized away to just directly reference the port. Co-authored-by: Morten Borup Petersen <[email protected]>
llvm · Jan 21, 2025 · 443f007 · 443f007
1 parent 6cfac8c
commit 443f007
Show file tree

Hide file tree

Showing 30 changed files with 4 additions and 337 deletions.
diff --git a/include/circt/Dialect/Kanagawa/KanagawaInterfaces.td b/include/circt/Dialect/Kanagawa/KanagawaInterfaces.td
@@ -68,8 +68,7 @@ def PortOpInterface : OpInterface<"PortOpInterface", [NamedInnerSymbol]> {
 def ScopeOpInterface : OpInterface<"ScopeOpInterface", [NamedInnerSymbol]> {
   let cppNamespace = "circt::kanagawa";
   let description = [{
-    An interface for operations which define Kanagawa scopes, that can be referenced
-    by an `kanagawa.this` operation.
+    An interface for operations which define Kanagawa scopes.
   }];
 
   let verify = "return detail::verifyScopeOpInterface($_op);";
@@ -82,20 +81,6 @@ def ScopeOpInterface : OpInterface<"ScopeOpInterface", [NamedInnerSymbol]> {
         return $_op.getBodyBlock();
       }]
     >,
-    InterfaceMethod<
-      "Returns `%this` of the scope",
-      "mlir::TypedValue<ScopeRefType>", "getThis",
-      (ins), [{
-        return *detail::getThisFromScope($_op);
-      }]
-    >,
-    InterfaceMethod<
-      "Returns the `ThisOp` of this scope",
-      "Operation*", "getThisOp",
-      (ins), [{
-        return cast<ScopeOpInterface>($_op.getOperation()).getThis().getDefiningOp();
-      }]
-    >,
     InterfaceMethod<
       "Returns the symbol name of the scope",
       "mlir::StringAttr", "getScopeName",

diff --git a/include/circt/Dialect/Kanagawa/KanagawaOps.h b/include/circt/Dialect/Kanagawa/KanagawaOps.h
@@ -30,7 +30,6 @@
 namespace circt {
 namespace kanagawa {
 class ContainerOp;
-class ThisOp;
 
 // Symbol name for the kanagawa operator library to be used during scheduling.
 static constexpr const char *kKanagawaOperatorLibName =
@@ -40,11 +39,6 @@ namespace detail {
 // Verify that `op` conforms to the ScopeOpInterface.
 LogicalResult verifyScopeOpInterface(Operation *op);
 
-// Returns the %this value of an kanagawa scope-defining operation. Implemented
-// here to hide the dependence on `kanagawa.this`, which is not defined before
-// the interface definition.
-mlir::FailureOr<mlir::TypedValue<ScopeRefType>> getThisFromScope(Operation *op);
-
 } // namespace detail
 } // namespace kanagawa
 } // namespace circt

diff --git a/include/circt/Dialect/Kanagawa/KanagawaOps.td b/include/circt/Dialect/Kanagawa/KanagawaOps.td
@@ -539,35 +539,6 @@ def PathOp : KanagawaOp<"path", [
   let hasCanonicalizeMethod = 1;
 }
 
-def ThisOp : KanagawaOp<"this", [
-  DeclareOpInterfaceMethods<InnerRefUserOpInterface>,
-  HasCustomSSAName
-]> {
-  let summary = "Return a handle to the current scope `!kanagawa.scoperef`";
-  let arguments = (ins InnerRefAttr:$scopeName);
-  let results = (outs ScopeRefType:$thisRef);
-
-  let assemblyFormat = [{
-    $scopeName attr-dict custom<ScopeRefFromName>(type($thisRef), ref($scopeName))
-  }];
-
-  let builders = [
-    OpBuilder<(ins "StringAttr":$moduleName, "hw::InnerSymAttr":$sym), [{
-      auto name = $_builder.getAttr<hw::InnerRefAttr>(
-        FlatSymbolRefAttr::get(moduleName), sym.getSymName());
-      build($_builder, $_state, $_builder.getType<ScopeRefType>(name), name);
-    }]>,
-    OpBuilder<(ins "hw::InnerRefAttr":$name), [{
-      build($_builder, $_state, $_builder.getType<ScopeRefType>(name), name);
-    }]>
-  ];
-
-  let extraClassDeclaration = [{
-    /// Return the scope operation which this accelerator refers to
-    ScopeOpInterface getScope(const hw::InnerRefNamespace &symbolTable);
-  }];
-}
-
 // ===---------------------------------------------------------------------===//
 // Low-level Kanagawa operations
 // ===---------------------------------------------------------------------===//
@@ -713,7 +684,6 @@ def GetPortOp : KanagawaOp<"get_port", [
     }]>
   ];
 
-  let hasCanonicalizeMethod = 1;
   let extraClassDeclaration = [{
     // Returns the direction requested by this get_port op.
     kanagawa::Direction getDirection() {

diff --git a/integration_test/Dialect/Ibis/end_to_end.mlir b/integration_test/Dialect/Ibis/end_to_end.mlir
@@ -7,24 +7,19 @@ kanagawa.design @foo {
 // A class hierarchy with a shared parent, and accessing between the children
 
 kanagawa.class sym @C1 {
-  %this = kanagawa.this <@foo::@C1>
   %out = kanagawa.port.output "out" sym @out : i32
   %c0 = hw.constant 42 : i32
   kanagawa.port.write %out, %c0 : !kanagawa.portref<out i32>
 }
 
 kanagawa.class sym @C2 {
-  %this = kanagawa.this <@foo::@C2>
-
   %go_port = kanagawa.port.input "go" sym @go : i1
   %clk_port = kanagawa.port.input "clk" sym @clk : !seq.clock
   %rst_port = kanagawa.port.input "rst" sym @rst : i1
   %done_port = kanagawa.port.output "done" sym @done : i1
   %out_port = kanagawa.port.output "out" sym @out : i32
 
   kanagawa.container sym @MyMethod {
-    %t = kanagawa.this <@foo::@MyMethod>
-
     // Grab parent go, clk, reset inputs - note that the requested direction of
     // these are flipped wrt. the defined direction of the ports. The semantics
     // are now that get_port defines the intended usage of the port (in => i'll write to the port, out => i'll read from the port).
@@ -69,7 +64,6 @@ kanagawa.class sym @C2 {
 }
 
 kanagawa.class sym @Parent {
-  %this = kanagawa.this <@foo::@Parent>
   %c1 = kanagawa.instance @c1, <@foo::@C1>
   %c2 = kanagawa.instance @c2, <@foo::@C2>
 

diff --git a/lib/Dialect/Kanagawa/KanagawaOps.cpp b/lib/Dialect/Kanagawa/KanagawaOps.cpp
@@ -43,7 +43,6 @@ static llvm::raw_string_ostream &genValueName(llvm::raw_string_ostream &os,
   auto *definingOp = value.getDefiningOp();
   assert(definingOp && "scoperef should always be defined by some op");
   llvm::TypeSwitch<Operation *, void>(definingOp)
-      .Case<ThisOp>([&](auto op) { os << "this"; })
       .Case<InstanceOp, ContainerInstanceOp>(
           [&](auto op) { os << op.getInstanceNameAttr().strref(); })
       .Case<PortOpInterface>([&](auto op) { os << op.getNameHint(); })
@@ -84,23 +83,7 @@ static StringAttr genValueNameAttr(Value v) {
 // ScopeOpInterface
 //===----------------------------------------------------------------------===//
 
-FailureOr<mlir::TypedValue<ScopeRefType>>
-circt::kanagawa::detail::getThisFromScope(Operation *op) {
-  auto scopeOp = cast<ScopeOpInterface>(op);
-  auto thisOps = scopeOp.getBodyBlock()->getOps<kanagawa::ThisOp>();
-  if (thisOps.empty())
-    return op->emitOpError("must contain a 'kanagawa.this' operation");
-
-  if (std::next(thisOps.begin()) != thisOps.end())
-    return op->emitOpError("must contain only one 'kanagawa.this' operation");
-
-  return (*thisOps.begin()).getThisRef();
-}
-
 LogicalResult circt::kanagawa::detail::verifyScopeOpInterface(Operation *op) {
-  if (failed(getThisFromScope(op)))
-    return failure();
-
   if (!isa<hw::InnerSymbolOpInterface>(op))
     return op->emitOpError("must implement 'InnerSymbolOpInterface'");
 
@@ -322,46 +305,10 @@ PortOpInterface GetPortOp::getPort(const hw::InnerRefNamespace &ns) {
       targetScope.lookupInnerSym(getPortSymbol()));
 }
 
-LogicalResult GetPortOp::canonicalize(GetPortOp op, PatternRewriter &rewriter) {
-  // Canonicalize away get_port on %this in favor of using the port SSA value
-  // directly.
-  // get_port(%this, @P) -> kanagawa.port.#
-  auto parentScope = dyn_cast<ScopeOpInterface>(op->getParentOp());
-  if (parentScope) {
-    auto scopeThis = parentScope.getThis();
-    if (op.getInstance() == scopeThis) {
-      auto definingPort = parentScope.lookupPort(op.getPortSymbol());
-      rewriter.replaceOp(op, {definingPort.getPort()});
-      return success();
-    }
-  }
-
-  return failure();
-}
-
 void GetPortOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
   setNameFn(getResult(), genValueNameAttr(getResult()));
 }
 
-//===----------------------------------------------------------------------===//
-// ThisOp
-//===----------------------------------------------------------------------===//
-
-LogicalResult ThisOp::verifyInnerRefs(hw::InnerRefNamespace &ns) {
-  if (!getScope(ns))
-    return emitOpError() << "'" << getScopeName() << "' does not exist";
-
-  return success();
-}
-
-void ThisOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
-  setNameFn(getResult(), "this");
-}
-
-ScopeOpInterface ThisOp::getScope(const hw::InnerRefNamespace &ns) {
-  return ns.lookupOp<ScopeOpInterface>(getScopeNameAttr());
-}
-
 //===----------------------------------------------------------------------===//
 // PortReadOp
 //===----------------------------------------------------------------------===//

diff --git a/lib/Dialect/Kanagawa/Transforms/KanagawaContainerize.cpp b/lib/Dialect/Kanagawa/Transforms/KanagawaContainerize.cpp
@@ -58,15 +58,6 @@ struct OutlineContainerPattern : public OpConversionPattern<ContainerOp> {
 
     rewriter.mergeBlocks(op.getBodyBlock(), newContainer.getBodyBlock(), {});
 
-    // Rename the kanagawa.this operation to refer to the proper op.
-    auto thisOp =
-        cast<ThisOp>(cast<ScopeOpInterface>(*newContainer.getOperation())
-                         .getThis()
-                         .getDefiningOp());
-    rewriter.setInsertionPoint(thisOp);
-    rewriter.replaceOpWithNewOp<ThisOp>(thisOp, design.getSymNameAttr(),
-                                        newContainer.getInnerSymAttr());
-
     // Create a container instance op in the parent class.
     rewriter.setInsertionPoint(op);
     rewriter.create<ContainerInstanceOp>(

diff --git a/lib/Dialect/Kanagawa/Transforms/KanagawaContainersToHW.cpp b/lib/Dialect/Kanagawa/Transforms/KanagawaContainersToHW.cpp
@@ -193,19 +193,6 @@ struct ContainerOpConversionPattern : public OpConversionPattern<ContainerOp> {
   ContainerHWModSymbolMap &modSymMap;
 };
 
-struct ThisOpConversionPattern : public OpConversionPattern<ThisOp> {
-  ThisOpConversionPattern(MLIRContext *ctx)
-      : OpConversionPattern<ThisOp>(ctx) {}
-
-  LogicalResult
-  matchAndRewrite(ThisOp op, OpAdaptor adaptor,
-                  ConversionPatternRewriter &rewriter) const override {
-    // TODO: remove this op from the dialect - not needed anymore.
-    rewriter.eraseOp(op);
-    return success();
-  }
-};
-
 struct ContainerInstanceOpConversionPattern
     : public OpConversionPattern<ContainerInstanceOp> {
 
@@ -369,7 +356,7 @@ void ContainersToHWPass::runOnOperation() {
   modSymCache.addDefinitions(getOperation());
   Namespace modNamespace;
   modNamespace.add(modSymCache);
-  target.addIllegalOp<ContainerOp, ContainerInstanceOp, ThisOp>();
+  target.addIllegalOp<ContainerOp, ContainerInstanceOp>();
   target.markUnknownOpDynamicallyLegal([](Operation *) { return true; });
 
   // Remove the name of the kanagawa.design's from the namespace - The
@@ -390,7 +377,6 @@ void ContainersToHWPass::runOnOperation() {
   patterns.add<ContainerOpConversionPattern>(ctx, modNamespace, portOrder,
                                              modSymMap);
   patterns.add<ContainerInstanceOpConversionPattern>(ctx, portOrder, modSymMap);
-  patterns.add<ThisOpConversionPattern>(ctx);
 
   if (failed(
           applyPartialConversion(getOperation(), target, std::move(patterns))))

diff --git a/lib/Dialect/Kanagawa/Transforms/KanagawaMethodsToContainers.cpp b/lib/Dialect/Kanagawa/Transforms/KanagawaMethodsToContainers.cpp
@@ -44,11 +44,6 @@ struct DataflowMethodOpConversion
         op.getLoc(), op.getInnerSym(), /*isTopLevel=*/false);
     rewriter.setInsertionPointToStart(newContainer.getBodyBlock());
 
-    // Create mandatory %this
-    // TODO @mortbopet: this will most likely already be present at the
-    // method.df level soon...
-    rewriter.create<ThisOp>(op.getLoc(), newContainer.getInnerRef());
-
     // Create in- and output ports.
     llvm::SmallVector<Value> argValues;
     for (auto [arg, name] : llvm::zip_equal(

diff --git a/test/Dialect/Kanagawa/Transforms/argify_blocks.mlir b/test/Dialect/Kanagawa/Transforms/argify_blocks.mlir
@@ -1,7 +1,6 @@
 // RUN: circt-opt --kanagawa-argify-blocks %s | FileCheck %s
 
 // CHECK-LABEL: kanagawa.class sym @Argify {
-// CHECK-NEXT:   %this = kanagawa.this <@foo::@Argify> 
 // CHECK-NEXT:   kanagawa.method @foo() -> () {
 // CHECK-NEXT:     %c32_i32 = hw.constant 32 : i32
 // CHECK-NEXT:     %0:2 = kanagawa.sblock.isolated (%arg0 : i32 = %c32_i32) -> (i32, i32) {
@@ -15,7 +14,6 @@
 
 kanagawa.design @foo {
 kanagawa.class sym @Argify {
-  %this = kanagawa.this <@foo::@Argify>
 
   kanagawa.method @foo()  {
     %c32 = hw.constant 32 : i32

diff --git a/test/Dialect/Kanagawa/Transforms/cf_to_handshake.mlir b/test/Dialect/Kanagawa/Transforms/cf_to_handshake.mlir
@@ -41,7 +41,6 @@
 
 kanagawa.design @foo {
 kanagawa.class sym @ToHandshake {
-  %this = kanagawa.this <@foo::@ToHandshake>
   // Just a simple test demonstrating the intended mixing of `kanagawa.sblock`s and
   // control flow operations. The meat of cf-to-handshake conversion is tested
   // in the handshake dialect tests.

diff --git a/test/Dialect/Kanagawa/Transforms/clean_selfdrivers.mlir b/test/Dialect/Kanagawa/Transforms/clean_selfdrivers.mlir
@@ -2,14 +2,12 @@
 
 kanagawa.design @D {
 // CHECK-LABEL:   kanagawa.container sym @C {
-// CHECK:           %[[VAL_0:.*]] = kanagawa.this <@D::@C>
 // CHECK:           %[[VAL_1:.*]] = hw.wire %[[VAL_2:.*]]  : i1
 // CHECK:           %[[VAL_3:.*]] = kanagawa.port.output "out" sym @out : i1
 // CHECK:           %[[VAL_2]] = hw.constant true
 // CHECK:           kanagawa.port.write %[[VAL_3]], %[[VAL_1]] : !kanagawa.portref<out i1>
 // CHECK:         }
 kanagawa.container sym @C {
-    %this = kanagawa.this <@D::@C>
     %in = kanagawa.port.input "in" sym @in : i1
     %out = kanagawa.port.output "out" sym @out : i1
     %true = hw.constant 1 : i1
@@ -24,29 +22,25 @@ kanagawa.container sym @C {
 
 kanagawa.design @D {
 // CHECK-LABEL:   kanagawa.container sym @Selfdriver {
-// CHECK:           %[[VAL_0:.*]] = kanagawa.this <@D::@Selfdriver>
 // CHECK:           %[[VAL_1:.*]] = hw.wire %[[VAL_2:.*]]  : i1
 // CHECK:           %[[VAL_3:.*]] = kanagawa.port.output "myIn" sym @in : i1
 // CHECK:           kanagawa.port.write %[[VAL_3]], %[[VAL_1]] : !kanagawa.portref<out i1>
 // CHECK:           %[[VAL_2]] = hw.constant true
 // CHECK:         }
 
 // CHECK-LABEL:   kanagawa.container sym @ParentReader {
-// CHECK:           %[[VAL_0:.*]] = kanagawa.this <@D::@ParentReader>
 // CHECK:           %[[VAL_1:.*]] = kanagawa.container.instance @selfdriver, <@D::@Selfdriver>
 // CHECK:           %[[VAL_2:.*]] = kanagawa.get_port %[[VAL_1]], @in : !kanagawa.scoperef<@D::@Selfdriver> -> !kanagawa.portref<out i1>
 // CHECK:           %[[VAL_3:.*]] = kanagawa.port.read %[[VAL_2]] : !kanagawa.portref<out i1>
 // CHECK:         }
 
 kanagawa.container sym @Selfdriver {
-  %this = kanagawa.this <@D::@Selfdriver>
   %in = kanagawa.port.input "myIn" sym @in : i1
   %true = hw.constant 1 : i1
   kanagawa.port.write %in, %true : !kanagawa.portref<in i1>
 }
 
 kanagawa.container sym @ParentReader {
-  %this = kanagawa.this <@D::@ParentReader>
   %selfdriver = kanagawa.container.instance @selfdriver, <@D::@Selfdriver>
   %in_ref = kanagawa.get_port %selfdriver, @in : !kanagawa.scoperef<@D::@Selfdriver> -> !kanagawa.portref<out i1>
   %in = kanagawa.port.read %in_ref : !kanagawa.portref<out i1>
@@ -59,20 +53,17 @@ kanagawa.container sym @ParentReader {
 kanagawa.design @D {
 
 kanagawa.container sym @Foo {
-  %this = kanagawa.this <@D::@Foo>
   %in = kanagawa.port.input "in" sym @in : i1
 }
 
 // CHECK-LABEL:   kanagawa.container sym @ParentReaderWriter {
-// CHECK:           %[[VAL_0:.*]] = kanagawa.this <@D::@ParentReaderWriter>
 // CHECK:           %[[VAL_1:.*]] = kanagawa.container.instance @f, <@D::@Foo>
 // CHECK:           %[[VAL_2:.*]] = kanagawa.get_port %[[VAL_1]], @in : !kanagawa.scoperef<@D::@Foo> -> !kanagawa.portref<in i1>
 // CHECK:           "foo.bar"(%[[VAL_3:.*]]) : (i1) -> ()
 // CHECK:           %[[VAL_3]] = hw.constant true
 // CHECK:           kanagawa.port.write %[[VAL_2]], %[[VAL_3]] : !kanagawa.portref<in i1>
 // CHECK:         }
 kanagawa.container sym @ParentReaderWriter {
-  %this = kanagawa.this <@D::@ParentReaderWriter>
   %f = kanagawa.container.instance @f, <@D::@Foo>
   %in_wr_ref = kanagawa.get_port %f, @in : !kanagawa.scoperef<@D::@Foo> -> !kanagawa.portref<in i1>
   %in_rd_ref = kanagawa.get_port %f, @in : !kanagawa.scoperef<@D::@Foo> -> !kanagawa.portref<out i1>

diff --git a/test/Dialect/Kanagawa/Transforms/containerize.mlir b/test/Dialect/Kanagawa/Transforms/containerize.mlir
@@ -7,7 +7,6 @@ kanagawa.design @foo {
 // CHECK-LABEL:   kanagawa.container sym @A_B_0
 // CHECK-LABEL:   kanagawa.container sym @A_C
 // CHECK-LABEL:   kanagawa.container sym @A {
-// CHECK:           %[[VAL_0:.*]] = kanagawa.this <@foo::@A>
 // CHECK:           kanagawa.port.input "A_in" sym @A_in : i1
 // CHECK:           %[[VAL_1:.*]] = kanagawa.container.instance @myClass, <@foo::@MyClass>
 // CHECK:           %[[VAL_2:.*]] = kanagawa.container.instance @A_B_0, <@foo::@A_B_0>
@@ -16,22 +15,17 @@ kanagawa.design @foo {
 // This container will alias with the @B inside @A, and thus checks the
 // name uniquing logic.
 kanagawa.container sym @A_B {
-  %this = kanagawa.this <@foo::@A_B>
 }
 
 kanagawa.class "MyClassName" sym @MyClass {
-  %this = kanagawa.this <@foo::@MyClass>
 }
 
 kanagawa.class sym @A {
-  %this = kanagawa.this <@foo::@A>
   kanagawa.port.input "A_in" sym @A_in : i1
   %myClass = kanagawa.instance @myClass, <@foo::@MyClass>
   kanagawa.container sym @B {
-    %B_this = kanagawa.this <@foo::@B>
   }
   kanagawa.container sym @C {
-    %C_this = kanagawa.this <@foo::@C>
   }
 }