[ScfToCf] Signedness-aware scf::ForOp lowering (#38)

This introduces a new conversion pass to Dynamatic++ (--lower-scf-to-cf)
that is almost identical to the upstream `--convert-scf-to-cf` pass,
which, as its name indicates, converts `scf` operations to unstructured
control flow (`cf`). The only difference between the two is that the
`scf::ForOp` rewrite pattern used in our pass (which overwrites the
default one) inserts an _unsigned_ comparison operation for checking
whether the loop iterator is within the for loop's bounds instead of a
_signed_ one if it can prove that both the iterator and upper bound are
guaranteed to be positive. The estimation relies on the recently
introduced numerical analysis. This has a performance impact
down-the-line during bitwidth analysis.

Fixes #28.

include/dynamatic/Conversion/Passes.h

@@ -9,6 +9,7 @@
 
 #include "dynamatic/Conversion/AffineToScf.h"
 #include "dynamatic/Conversion/HandshakeToNetlist.h"
+#include "dynamatic/Conversion/ScfToCf.h"
 #include "dynamatic/Conversion/StandardToHandshakeFPGA18.h"
 #include "mlir/IR/DialectRegistry.h"
 #include "mlir/Pass/Pass.h"

include/dynamatic/Conversion/Passes.td

@@ -30,6 +30,24 @@ def AffineToScf : Pass<"lower-affine-to-scf", "mlir::ModuleOp"> {
   ];
 }
 
+//===----------------------------------------------------------------------===//
+// ScfToCf
+//===----------------------------------------------------------------------===//
+
+def ScfToCf : Pass<"lower-scf-to-cf", "mlir::ModuleOp"> {
+  let summary = "Lower scf dialect to unstructured control flow (cf)";
+  let description = [{
+    Very close analog to the SCFToControlFlow pass from MLIR that replaces the
+    structured for loop lowering pattern with an almost identical one that
+    additionally attempts to insert an unsigned comparison (ult) in the IR
+    instead of a signed one (lt) if the loop's iterator can be proven to be
+    always positive.
+  }];
+  let constructor = "dynamatic::createLowerScfToCf()";
+  let dependentDialects = [
+    "mlir::cf::ControlFlowDialect", "mlir::arith::ArithDialect"];
+}
+
 //===----------------------------------------------------------------------===//
 // StandardToHandshakeFPGA18
 //===----------------------------------------------------------------------===//

include/dynamatic/Conversion/ScfToCf.h

@@ -0,0 +1,26 @@
+//===- ScfToCf.h - Lower scf ops to unstructured control flow ---*- C++ -*-===//
+//
+// This file declares the --lower-scf-to-cf pass.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef DYNAMATIC_TRANSFORMS_SCFTOCF_H
+#define DYNAMATIC_TRANSFORMS_SCFTOCF_H
+
+#include "dynamatic/Support/LLVM.h"
+#include "mlir/Dialect/Arith/IR/Arith.h"
+#include "mlir/Dialect/ControlFlow/IR/ControlFlowOps.h"
+#include "mlir/IR/DialectRegistry.h"
+#include "mlir/Pass/Pass.h"
+
+namespace dynamatic {
+
+#define GEN_PASS_DECL_SCFTOCF
+#define GEN_PASS_DEF_SCFTOCF
+#include "dynamatic/Conversion/Passes.h.inc"
+
+std::unique_ptr<mlir::OperationPass<mlir::ModuleOp>> createLowerScfToCf();
+
+} // namespace dynamatic
+
+#endif // DYNAMATIC_TRANSFORMS_SCFTOCF_H

lib/Conversion/CMakeLists.txt

@@ -1,3 +1,4 @@
 add_subdirectory(AffineToScf)
+add_subdirectory(ScfToCf)
 add_subdirectory(HandshakeToNetlist)
 add_subdirectory(StandardToHandshakeFPGA18)

lib/Conversion/ScfToCf/CMakeLists.txt

@@ -0,0 +1,15 @@
+add_dynamatic_library(DynamaticLowerScfToCf
+  ScfToCf.cpp
+
+  DEPENDS
+  DynamaticConversionPassIncGen
+
+  LINK_LIBS PUBLIC
+  MLIRIR
+  MLIRArithDialect
+  MLIRSCFDialect
+  MLIRPass
+  MLIRTransforms
+  MLIRSCFToControlFlow
+  DynamaticSupport
+  )

lib/Conversion/ScfToCf/ScfToCf.cpp

@@ -0,0 +1,136 @@
+//===- ScfToCf.cpp - Lower scf ops to unstructured control flow -*- C++ -*-===//
+//
+// Implements the --lower-scf-to-cf conversion pass. It is different from the
+// upstream --convert-scf-to-cf pass in only one respect: it basically
+// "overwrites" the for-lowering pattern from the upstream pass with a custom
+// one which inserts unsigned integer comparisons in the IR whenever possible;
+// this is in opposition to the for-lowering of the upstream pass, which always
+// inserts signed comparisons.
+//
+//===----------------------------------------------------------------------===//
+
+#include "dynamatic/Conversion/ScfToCf.h"
+#include "dynamatic/Analysis/NumericAnalysis.h"
+#include "mlir/Conversion/SCFToControlFlow/SCFToControlFlow.h"
+#include "mlir/Dialect/SCF/IR/SCF.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+
+using namespace mlir;
+using namespace dynamatic;
+
+namespace {
+
+/// Lower structured for loops into their unstructured form. Taken from MLIR's
+/// --convert-scf-to-cf pass, but creates an unsigned comparison (ult) instead
+/// of a signed one (lt) if the loop iterator can be proven to be always
+/// positive.
+struct ForLowering : public OpRewritePattern<scf::ForOp> {
+  using OpRewritePattern<scf::ForOp>::OpRewritePattern;
+
+  LogicalResult matchAndRewrite(scf::ForOp forOp,
+                                PatternRewriter &rewriter) const override {
+    Location loc = forOp.getLoc();
+
+    // Compute loop bounds before branching to the condition.
+    Value lowerBound = forOp.getLowerBound();
+    Value upperBound = forOp.getUpperBound();
+    if (!lowerBound || !upperBound)
+      return failure();
+
+    // Determine comparison predicate to use when lowering the loop. We can
+    // insert an unsigned comparison only if the lower bound can be guaranteed
+    // to be non-negative
+    NumericAnalysis analysis;
+    arith::CmpIPredicate pred = analysis.getRange(lowerBound).isPositive()
+                                    ? arith::CmpIPredicate::ult
+                                    : arith::CmpIPredicate::slt;
+
+    // Start by splitting the block containing the 'scf.for' into two parts.
+    // The part before will get the init code, the part after will be the end
+    // point.
+    auto *initBlock = rewriter.getInsertionBlock();
+    auto initPosition = rewriter.getInsertionPoint();
+    auto *endBlock = rewriter.splitBlock(initBlock, initPosition);
+
+    // Use the first block of the loop body as the condition block since it is
+    // the block that has the induction variable and loop-carried values as
+    // arguments. Split out all operations from the first block into a new
+    // block. Move all body blocks from the loop body region to the region
+    // containing the loop.
+    auto *conditionBlock = &forOp.getRegion().front();
+    auto *firstBodyBlock =
+        rewriter.splitBlock(conditionBlock, conditionBlock->begin());
+    auto *lastBodyBlock = &forOp.getRegion().back();
+    rewriter.inlineRegionBefore(forOp.getRegion(), endBlock);
+    auto iv = conditionBlock->getArgument(0);
+
+    // Append the induction variable stepping logic to the last body block and
+    // branch back to the condition block. Loop-carried values are taken from
+    // operands of the loop terminator.
+    Operation *terminator = lastBodyBlock->getTerminator();
+    rewriter.setInsertionPointToEnd(lastBodyBlock);
+    auto step = forOp.getStep();
+    auto stepped = rewriter.create<arith::AddIOp>(loc, iv, step).getResult();
+    if (!stepped)
+      return failure();
+
+    SmallVector<Value, 8> loopCarried;
+    loopCarried.push_back(stepped);
+    loopCarried.append(terminator->operand_begin(), terminator->operand_end());
+    rewriter.create<cf::BranchOp>(loc, conditionBlock, loopCarried);
+    rewriter.eraseOp(terminator);
+
+    // The initial values of loop-carried values is obtained from the operands
+    // of the loop operation.
+    SmallVector<Value, 8> destOperands;
+    destOperands.push_back(lowerBound);
+    auto iterOperands = forOp.getIterOperands();
+    destOperands.append(iterOperands.begin(), iterOperands.end());
+    rewriter.setInsertionPointToEnd(initBlock);
+    rewriter.create<cf::BranchOp>(loc, conditionBlock, destOperands);
+
+    // With the body block done, we can fill in the condition block.
+    rewriter.setInsertionPointToEnd(conditionBlock);
+    auto comparison = rewriter.create<arith::CmpIOp>(loc, pred, iv, upperBound);
+
+    rewriter.create<cf::CondBranchOp>(loc, comparison, firstBodyBlock,
+                                      ArrayRef<Value>(), endBlock,
+                                      ArrayRef<Value>());
+    // The result of the loop operation is the values of the condition block
+    // arguments except the induction variable on the last iteration.
+    rewriter.replaceOp(forOp, conditionBlock->getArguments().drop_front());
+    return success();
+  }
+};
+
+struct ScfToCfPass : public dynamatic::impl::ScfToCfBase<ScfToCfPass> {
+
+  void runOnOperation() override {
+    MLIRContext *ctx = &getContext();
+    ModuleOp modOp = getOperation();
+
+    // Set up rewrite patterns
+    RewritePatternSet patterns{ctx};
+    // Our for lowering is given a higher benefit than the one defined in MLIR,
+    // so it will be matched first, essentially overriding the default one
+    patterns.add<ForLowering>(ctx, /*benefit=*/2);
+    populateSCFToControlFlowConversionPatterns(patterns);
+
+    // Set up conversion target
+    ConversionTarget target(*ctx);
+    target.addIllegalOp<scf::ForOp, scf::IfOp, scf::ParallelOp, scf::WhileOp,
+                        scf::ExecuteRegionOp>();
+    target.markUnknownOpDynamicallyLegal([](Operation *) { return true; });
+
+    if (failed(applyPartialConversion(modOp, target, std::move(patterns))))
+      signalPassFailure();
+  };
+};
+} // namespace
+
+namespace dynamatic {
+std::unique_ptr<mlir::OperationPass<mlir::ModuleOp>> createLowerScfToCf() {
+  return std::make_unique<ScfToCfPass>();
+}
+} // namespace dynamatic

tools/dynamatic-opt/CMakeLists.txt

@@ -10,6 +10,7 @@ llvm_update_compile_flags(dynamatic-opt)
 target_link_libraries(dynamatic-opt
   PRIVATE
   AffineToScf
+  DynamaticLowerScfToCf
   DynamaticBufferPlacement
   DynamaticStandardToHandshakeFPGA18
   DynamaticHandshakeToNetlist