Remove the default lowering of instance choice

llvm · Dec 4, 2024 · 10b0ec9 · 10b0ec9
1 parent fa61aee
commit 10b0ec9
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 20 deletions.
diff --git a/include/circt/Dialect/FIRRTL/Passes.h b/include/circt/Dialect/FIRRTL/Passes.h
@@ -202,8 +202,8 @@ std::unique_ptr<mlir::Pass> createProbesToSignalsPass();
 
 std::unique_ptr<mlir::Pass> createSpecializeLayersPass();
 
-std::unique_ptr<mlir::Pass> createSpecializeOptionPass(
-    std::optional<mlir::ArrayRef<std::string>> selectOption = {});
+std::unique_ptr<mlir::Pass>
+createSpecializeOptionPass(mlir::ArrayRef<std::string> selectOption = {});
 
 std::unique_ptr<mlir::Pass> createCreateCompanionAssume();
 

diff --git a/lib/Dialect/FIRRTL/Transforms/SpecializeOption.cpp b/lib/Dialect/FIRRTL/Transforms/SpecializeOption.cpp
@@ -64,21 +64,20 @@ struct SpecializeOptionPass
         &getContext(), circuit.getOps<FModuleOp>(), [&](auto module) {
           module.walk([&](firrtl::InstanceChoiceOp inst) {
             auto it = selected.find(inst.getOptionNameAttr());
-            FlatSymbolRefAttr target;
             if (it == selected.end()) {
-              target = inst.getDefaultTargetAttr();
-              inst->emitWarning("missing specialization for option ")
+              inst.emitError("missing specialization for option ")
                   << inst.getOptionNameAttr();
-            } else
-              target = inst.getTargetOrDefaultAttr(it->second);
+              failed = true;
+              return;
+            }
 
             ImplicitLocOpBuilder builder(inst.getLoc(), inst);
             auto newInst = builder.create<InstanceOp>(
-                inst->getResultTypes(), target, inst.getNameAttr(),
-                inst.getNameKindAttr(), inst.getPortDirectionsAttr(),
-                inst.getPortNamesAttr(), inst.getAnnotationsAttr(),
-                inst.getPortAnnotationsAttr(), builder.getArrayAttr({}),
-                UnitAttr{}, inst.getInnerSymAttr());
+                inst->getResultTypes(), inst.getTargetOrDefaultAttr(it->second),
+                inst.getNameAttr(), inst.getNameKindAttr(),
+                inst.getPortDirectionsAttr(), inst.getPortNamesAttr(),
+                inst.getAnnotationsAttr(), inst.getPortAnnotationsAttr(),
+                builder.getArrayAttr({}), UnitAttr{}, inst.getInnerSymAttr());
             inst.replaceAllUsesWith(newInst);
             inst.erase();
 
@@ -100,10 +99,10 @@ struct SpecializeOptionPass
 };
 } // namespace
 
-std::unique_ptr<Pass> firrtl::createSpecializeOptionPass(
-    std::optional<mlir::ArrayRef<std::string>> selectOption) {
+std::unique_ptr<Pass>
+firrtl::createSpecializeOptionPass(mlir::ArrayRef<std::string> selectOption) {
   auto pass = std::make_unique<SpecializeOptionPass>();
-  if (selectOption)
-    pass->select = *selectOption;
+  if (!selectOption.empty())
+    pass->select = selectOption;
   return pass;
 }
diff --git a/test/Dialect/FIRRTL/specialize-option-omit.mlir b/test/Dialect/FIRRTL/specialize-option-omit.mlir
@@ -22,8 +22,7 @@ firrtl.module @Foo() {
   firrtl.instance_choice inst_default @DefaultTarget alternatives @Platform
     { @ASIC -> @ASICTarget } ()
 
-  // CHECK: firrtl.instance inst_perf @DefaultTarget()
-  // expected-warning @below {{missing specialization for option "Performance"}}
+  // expected-error @below {{missing specialization for option "Performance"}}
   firrtl.instance_choice inst_perf @DefaultTarget alternatives @Performance
     { @Fast -> @FastTarget } ()
 }

diff --git a/test/firtool/instchoice.fir b/test/firtool/instchoice.fir
@@ -1,6 +1,5 @@
 ; RUN: firtool %s --parse-only | FileCheck %s
 ; RUN: firtool %s --ir-hw --disable-opt --select-instance-choice=Platform=ASIC | FileCheck %s --check-prefix=ASIC
-; RUN: firtool %s --ir-hw --disable-opt | FileCheck %s --check-prefix=DEFAULT
 
 FIRRTL version 4.1.0
 circuit Foo:
@@ -29,7 +28,6 @@ circuit Foo:
     ; CHECK-SAME:   @ASIC -> @ASICTarget
     ; CHECK-SAME: } (in clock: !firrtl.clock)
     ; ASIC: hw.instance "inst" @ASICTarget(clock: %clock: !seq.clock) -> ()
-    ; DEFAULT: hw.instance "inst" @DefaultTarget(clock: %clock: !seq.clock) -> ()
     instchoice inst of DefaultTarget, Platform :
       FPGA => FPGATarget
       ASIC => ASICTarget