Add results to sv.ifdef

include/circt/Dialect/SV/SVStatements.td

@@ -24,6 +24,59 @@ include "circt/Support/ProceduralRegionTrait.td"
 // Control flow like-operations
 //===----------------------------------------------------------------------===//
 
+def YieldOp : SVOp<"yield", [Pure, ReturnLike, Terminator,
+    ParentOneOf<["IfDefOp", "IfDefProceduralOp"]>]> {
+  let summary = "yield values from SV ifdef operations";
+  let description = [{
+    The `sv.yield` operation yields SSA values from an SV ifdef operation's
+    region and terminates the region. The parent ifdef operation concatenates
+    the results from the then region with the results from the else region.
+
+    Each region can yield a different number and types of values. The yielded
+    values from each region become results of the parent operation in order:
+    first all values from the then region, then all values from the else region.
+
+    If the parent operation defines no values, then the `sv.yield` may be
+    left out in the custom syntax and the builders will insert one implicitly.
+    Otherwise, it has to be present in the syntax to indicate which values are
+    yielded.
+
+    Example with different types from each region:
+    ```mlir
+    // Returns 2 results: i32 from then, i64 from else
+    %0:2 = sv.ifdef @MACRO -> i32, i64 {
+      %then_val = ...
+      sv.yield %then_val : i32
+    } else {
+      %else_val = ...
+      sv.yield %else_val : i64
+    }
+    ```
+
+    Example with multiple values from one region:
+    ```mlir
+    // Returns 3 results: 2 from then, 1 from else
+    %0:3 = sv.ifdef @MACRO -> i32, i64, i8 {
+      %v1 = ...
+      %v2 = ...
+      sv.yield %v1, %v2 : i32, i64
+    } else {
+      %v3 = ...
+      sv.yield %v3 : i8
+    }
+    ```
+  }];
+
+  let arguments = (ins Variadic<AnyType>:$results);
+  let assemblyFormat = "($results^ `:` type($results))? attr-dict";
+
+  let builders = [
+    OpBuilder<(ins), [{
+      build($_builder, $_state, mlir::ValueRange());
+    }]>
+  ];
+}
+
 def OrderedOutputOp : SVOp<"ordered", [SingleBlock, NoTerminator, NoRegionArguments,
                              NonProceduralOp]> {
   let summary = "a sub-graph region which guarantees to output statements in-order";
@@ -50,22 +103,61 @@ def IfDefOp : SVOp<"ifdef", [
     NoRegionArguments,
     NonProceduralOp,
     NonProceduralRegion,
-    GraphRegionNoTerminator,
+    SingleBlockImplicitTerminator<"YieldOp">,
+    RegionKindInterface,
+    HasOnlyGraphRegion,
     DeclareOpInterfaceMethods<SymbolUserOpInterface>
 ]> {
   let summary = "'ifdef MACRO' block";
 
   let description = [{
     This operation is an #ifdef block, which has a "then" and "else" region.
     This operation is for non-procedural regions and its body is non-procedural.
+
+    The `sv.ifdef` operation may produce results. The results are formed by
+    concatenating the values yielded from the then region followed by the values
+    yielded from the else region. The then and else regions can yield different
+    numbers and types of values.
+
+    Example without results:
+    ```mlir
+    sv.ifdef @MACRO {
+      ...
+    } else {
+      ...
+    }
+    ```
+
+    Example with results from both regions:
+    ```mlir
+    // Returns 2 results: first from then, second from else
+    %0:2 = sv.ifdef @MACRO -> i32, i64 {
+      %then_val = ...
+      sv.yield %then_val : i32
+    } else {
+      %else_val = ...
+      sv.yield %else_val : i64
+    }
+    ```
+
+    Example with only then region having results:
+    ```mlir
+    %0 = sv.ifdef @MACRO -> i32 {
+      %value = ...
+      sv.yield %value : i32
+    } else {
+      sv.yield
+    }
+    ```
   }];
 
   let regions = (region SizedRegion<1>:$thenRegion, AnyRegion:$elseRegion);
   let arguments = (ins MacroIdentAttr:$cond);
-  let results = (outs);
+  let results = (outs Variadic<AnyType>:$results);
 
   let hasCanonicalizeMethod = true;
-  let assemblyFormat = "$cond $thenRegion (`else` $elseRegion^)? attr-dict";
+  let hasVerifier = 1;
+  let hasCustomAssemblyFormat = 1;
 
   // TODO: ODS forces using a custom builder just to get the region terminator
   // implicitly installed.
@@ -81,6 +173,9 @@ def IfDefOp : SVOp<"ifdef", [
                       CArg<"std::function<void()>", "{}">:$thenCtor,
                       CArg<"std::function<void()>", "{}">:$elseCtor)>,
     OpBuilder<(ins "MacroIdentAttr":$cond,
+                      CArg<"std::function<void()>", "{}">:$thenCtor,
+                      CArg<"std::function<void()>", "{}">:$elseCtor)>,
+    OpBuilder<(ins "TypeRange":$resultTypes, "MacroIdentAttr":$cond,
                       CArg<"std::function<void()>", "{}">:$thenCtor,
                       CArg<"std::function<void()>", "{}">:$elseCtor)>
   ];
@@ -98,12 +193,31 @@ def IfDefOp : SVOp<"ifdef", [
       assert(hasElse() && "Empty 'else' region.");
       return &getElseRegion().front();
     }
+
+    /// Get the number of results from the then region.
+    unsigned getNumThenResults();
+
+    /// Get the number of results from the else region.
+    unsigned getNumElseResults();
+
+    /// Get results from the then region.
+    mlir::ResultRange getThenResults() {
+      return getResults().take_front(getNumThenResults());
+    }
+
+    /// Get results from the else region.
+    mlir::ResultRange getElseResults() {
+      return getResults().drop_front(getNumThenResults());
+    }
+
+    // Implement RegionKindInterface.
+    static RegionKind getRegionKind(unsigned index) { return RegionKind::Graph; }
   }];
 }
 
 def IfDefProceduralOp : SVOp<"ifdef.procedural", [
       SingleBlock,
-      NoTerminator,
+      SingleBlockImplicitTerminator<"YieldOp">,
       NoRegionArguments,
       ProceduralRegion,
       ProceduralOp,
@@ -114,14 +228,44 @@ def IfDefProceduralOp : SVOp<"ifdef.procedural", [
   let description = [{
     This operation is an #ifdef block, which has a "then" and "else" region.
     This operation is for procedural regions and its body is procedural.
+
+    The `sv.ifdef.procedural` operation may produce results. The results are
+    formed by concatenating the values yielded from the then region followed by
+    the values yielded from the else region. The then and else regions can yield
+    different numbers and types of values.
+
+    Example without results:
+    ```mlir
+    sv.initial {
+      sv.ifdef.procedural @MACRO {
+        ...
+      } else {
+        ...
+      }
+    }
+    ```
+
+    Example with results from both regions:
+    ```mlir
+    sv.initial {
+      // Returns 2 results: first from then, second from else
+      %0:2 = sv.ifdef.procedural @MACRO -> i32, i64 {
+        %then_val = ...
+        sv.yield %then_val : i32
+      } else {
+        %else_val = ...
+        sv.yield %else_val : i64
+      }
+    }
+    ```
   }];
 
   let hasCanonicalizeMethod = true;
+  let hasVerifier = 1;
   let regions = (region SizedRegion<1>:$thenRegion, AnyRegion:$elseRegion);
   let arguments = (ins MacroIdentAttr:$cond);
-  let results = (outs);
-
-  let assemblyFormat = "$cond $thenRegion (`else` $elseRegion^)? attr-dict";
+  let results = (outs Variadic<AnyType>:$results);
+  let hasCustomAssemblyFormat = 1;
 
   // TODO: ODS forces using a custom builder just to get the region terminator
   // implicitly installed.
@@ -137,6 +281,9 @@ def IfDefProceduralOp : SVOp<"ifdef.procedural", [
                       CArg<"std::function<void()>", "{}">:$thenCtor,
                       CArg<"std::function<void()>", "{}">:$elseCtor)>,
     OpBuilder<(ins "MacroIdentAttr":$cond,
+                      CArg<"std::function<void()>", "{}">:$thenCtor,
+                      CArg<"std::function<void()>", "{}">:$elseCtor)>,
+    OpBuilder<(ins "TypeRange":$resultTypes, "MacroIdentAttr":$cond,
                       CArg<"std::function<void()>", "{}">:$thenCtor,
                       CArg<"std::function<void()>", "{}">:$elseCtor)>
   ];
@@ -154,6 +301,22 @@ def IfDefProceduralOp : SVOp<"ifdef.procedural", [
       assert(hasElse() && "Empty 'else' region.");
       return &getElseRegion().front();
     }
+
+    /// Get the number of results from the then region.
+    unsigned getNumThenResults();
+
+    /// Get the number of results from the else region.
+    unsigned getNumElseResults();
+
+    /// Get results from the then region.
+    mlir::ResultRange getThenResults() {
+      return getResults().take_front(getNumThenResults());
+    }
+
+    /// Get results from the else region.
+    mlir::ResultRange getElseResults() {
+      return getResults().drop_front(getNumThenResults());
+    }
   }];
 }
 

include/circt/Dialect/SV/SVVisitors.h

@@ -37,7 +37,7 @@ class Visitor {
             RegOp, WireOp, LogicOp, LocalParamOp, XMROp, XMRRefOp,
             // Control flow.
             OrderedOutputOp, IfDefOp, IfDefProceduralOp, IfOp, AlwaysOp,
-            AlwaysCombOp, AlwaysFFOp, InitialOp, CaseOp,
+            AlwaysCombOp, AlwaysFFOp, InitialOp, CaseOp, YieldOp,
             // Other Statements.
             AssignOp, BPAssignOp, PAssignOp, ForceOp, ReleaseOp, AliasOp,
             WriteOp, FWriteOp, FFlushOp, SystemFunctionOp, VerbatimOp,
@@ -128,6 +128,7 @@ class Visitor {
   HANDLE(AlwaysFFOp, Unhandled);
   HANDLE(InitialOp, Unhandled);
   HANDLE(CaseOp, Unhandled);
+  HANDLE(YieldOp, Unhandled);
 
   // Other Statements.
   HANDLE(AssignOp, Unhandled);

lib/Conversion/ExportVerilog/ExportVerilog.cpp

@@ -4087,6 +4087,7 @@ class StmtEmitter : public EmitterBase,
   LogicalResult visitSV(AlwaysFFOp op);
   LogicalResult visitSV(InitialOp op);
   LogicalResult visitSV(CaseOp op);
+  LogicalResult visitSV(YieldOp op);
   template <typename OpTy, typename EmitPrefixFn>
   LogicalResult
   emitFormattedWriteLikeOp(OpTy op, StringRef callee, StringRef formatString,
@@ -5254,12 +5255,21 @@ LogicalResult StmtEmitter::emitIfDef(Operation *op, MacroIdentAttr cond) {
   if (hasSVAttributes(op))
     emitError(op, "SV attributes emission is unimplemented for the op");
 
+  // Note: ifdef operations with results should be caught and rejected in the
+  // PrepareForEmission pass, so we don't need to check here.
+  assert(op->getNumResults() == 0 &&
+         "ifdef with results should be rejected in PrepareForEmission");
+
   auto ident = PPExtString(
       cast<MacroDeclOp>(state.symbolCache.getDefinition(cond.getIdent()))
           .getMacroIdentifier());
 
   startStatement();
-  bool hasEmptyThen = op->getRegion(0).front().empty();
+  // Check if the then block is empty (ignoring the implicit yield terminator)
+  auto &thenBlock = op->getRegion(0).front();
+  bool hasEmptyThen =
+      thenBlock.empty() || (thenBlock.getOperations().size() == 1 &&
+                            isa<YieldOp>(thenBlock.front()));
   if (hasEmptyThen)
     ps << "`ifndef " << ident;
   else
@@ -5289,6 +5299,15 @@ LogicalResult StmtEmitter::emitIfDef(Operation *op, MacroIdentAttr cond) {
   return success();
 }
 
+LogicalResult StmtEmitter::visitSV(YieldOp op) {
+  // sv.yield is a terminator and doesn't emit anything.
+  // Yields with operands (results) should be rejected in PrepareForEmission.
+  // Empty yields (for ifdef without results) are simply skipped.
+  assert(op.getNumOperands() == 0 &&
+         "yield with operands should be rejected in PrepareForEmission");
+  return success();
+}
+
 /// Emit the body of a control flow statement that is surrounded by begin/end
 /// markers if non-singular.  If the control flow construct is multi-line and
 /// if multiLineComment is non-null, the string is included in a comment after

lib/Conversion/ExportVerilog/PrepareForEmission.cpp

@@ -968,6 +968,44 @@ static LogicalResult legalizeHWModule(Block &block,
       continue;
     }
 
+    // Check for sv.yield operations that yield values, which should not exist.
+    // Empty yields (terminators for ifdef without results) are okay.
+    if (auto yieldOp = dyn_cast<YieldOp>(op)) {
+      if (yieldOp.getNumOperands() > 0) {
+        auto d = yieldOp.emitError()
+                 << "sv.yield operations with operands must be lowered before "
+                    "ExportVerilog";
+        d.attachNote() << "ExportVerilog does not support ifdef operations "
+                          "with results; they need to be lowered to temporary "
+                          "variables before emission";
+        return failure();
+      }
+    }
+
+    // Check for ifdef operations with results, which are not supported.
+    if (auto ifdefOp = dyn_cast<IfDefOp>(op)) {
+      if (ifdefOp.getNumResults() > 0) {
+        auto d = ifdefOp.emitError() << "sv.ifdef operations with results must "
+                                        "be lowered before ExportVerilog";
+        d.attachNote() << "ExportVerilog does not support ifdef operations "
+                          "with results; they need to be lowered to temporary "
+                          "variables before emission";
+        return failure();
+      }
+    }
+
+    if (auto ifdefProcOp = dyn_cast<IfDefProceduralOp>(op)) {
+      if (ifdefProcOp.getNumResults() > 0) {
+        auto d = ifdefProcOp.emitError()
+                 << "sv.ifdef.procedural operations with results must be "
+                    "lowered before ExportVerilog";
+        d.attachNote() << "ExportVerilog does not support ifdef operations "
+                          "with results; they need to be lowered to temporary "
+                          "variables before emission";
+        return failure();
+      }
+    }
+
     // Name legalization should have happened in a different pass for these sv
     // elements and we don't want to change their name through re-legalization
     // (e.g. letting a temporary take the name of an unvisited wire). Adding

lib/Conversion/FIRRTLToHW/LowerToHW.cpp

@@ -3157,7 +3157,12 @@ void FIRRTLLowering::runWithInsertionPointAtEndOfBlock(
 
   auto oldIP = builder.saveInsertionPoint();
 
-  builder.setInsertionPointToEnd(&region.front());
+  auto &block = region.front();
+  // If the block has a terminator, insert before it; otherwise at the end
+  if (!block.empty() && block.back().hasTrait<OpTrait::IsTerminator>())
+    builder.setInsertionPoint(&block.back());
+  else
+    builder.setInsertionPointToEnd(&block);
   fn();
   builder.restoreInsertionPoint(oldIP);
 }

lib/Conversion/SeqToSV/FirRegLowering.cpp

@@ -816,14 +816,19 @@ void FirRegLowering::buildRegConditions(OpBuilder &b, sv::RegOp reg) {
     if (kind == RegCondition::IfDefThen) {
       auto ifDef = sv::IfDefProceduralOp::create(b, reg.getLoc(),
                                                  condition.getMacro(), []() {});
-      b.setInsertionPointToEnd(ifDef.getThenBlock());
+      // Insert before the implicit terminator
+      auto *block = ifDef.getThenBlock();
+      b.setInsertionPoint(&block->back());
       continue;
     }
     if (kind == RegCondition::IfDefElse) {
       auto ifDef = sv::IfDefProceduralOp::create(
           b, reg.getLoc(), condition.getMacro(), []() {}, []() {});
 
-      b.setInsertionPointToEnd(ifDef.getElseBlock());
+      // Insert before the implicit terminator.
+      // The block will always have a terminator after create() returns.
+      auto *block = ifDef.getElseBlock();
+      b.setInsertionPoint(&block->back());
       continue;
     }
     llvm_unreachable("unknown reg condition type");