From 8db32e601b462dd7f725c4738bbae1d068ed9b83 Mon Sep 17 00:00:00 2001 From: Lucas Ramirez Date: Mon, 18 Sep 2023 22:42:16 +0200 Subject: [PATCH] [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 | 1 + include/dynamatic/Conversion/Passes.td | 18 ++++ include/dynamatic/Conversion/ScfToCf.h | 26 +++++ lib/Conversion/CMakeLists.txt | 1 + lib/Conversion/ScfToCf/CMakeLists.txt | 15 +++ lib/Conversion/ScfToCf/ScfToCf.cpp | 136 +++++++++++++++++++++++++ tools/dynamatic-opt/CMakeLists.txt | 1 + 7 files changed, 198 insertions(+) create mode 100644 include/dynamatic/Conversion/ScfToCf.h create mode 100644 lib/Conversion/ScfToCf/CMakeLists.txt create mode 100644 lib/Conversion/ScfToCf/ScfToCf.cpp diff --git a/include/dynamatic/Conversion/Passes.h b/include/dynamatic/Conversion/Passes.h index f3b41a5d0..6cbfd5827 100644 --- a/include/dynamatic/Conversion/Passes.h +++ b/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" diff --git a/include/dynamatic/Conversion/Passes.td b/include/dynamatic/Conversion/Passes.td index d2c16acd1..8db3f3465 100644 --- a/include/dynamatic/Conversion/Passes.td +++ b/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 //===----------------------------------------------------------------------===// diff --git a/include/dynamatic/Conversion/ScfToCf.h b/include/dynamatic/Conversion/ScfToCf.h new file mode 100644 index 000000000..ec01a6beb --- /dev/null +++ b/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> createLowerScfToCf(); + +} // namespace dynamatic + +#endif // DYNAMATIC_TRANSFORMS_SCFTOCF_H diff --git a/lib/Conversion/CMakeLists.txt b/lib/Conversion/CMakeLists.txt index bf597a922..55f208f1e 100644 --- a/lib/Conversion/CMakeLists.txt +++ b/lib/Conversion/CMakeLists.txt @@ -1,3 +1,4 @@ add_subdirectory(AffineToScf) +add_subdirectory(ScfToCf) add_subdirectory(HandshakeToNetlist) add_subdirectory(StandardToHandshakeFPGA18) diff --git a/lib/Conversion/ScfToCf/CMakeLists.txt b/lib/Conversion/ScfToCf/CMakeLists.txt new file mode 100644 index 000000000..d64da41e9 --- /dev/null +++ b/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 + ) diff --git a/lib/Conversion/ScfToCf/ScfToCf.cpp b/lib/Conversion/ScfToCf/ScfToCf.cpp new file mode 100644 index 000000000..bb046f55f --- /dev/null +++ b/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 { + using OpRewritePattern::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(loc, iv, step).getResult(); + if (!stepped) + return failure(); + + SmallVector loopCarried; + loopCarried.push_back(stepped); + loopCarried.append(terminator->operand_begin(), terminator->operand_end()); + rewriter.create(loc, conditionBlock, loopCarried); + rewriter.eraseOp(terminator); + + // The initial values of loop-carried values is obtained from the operands + // of the loop operation. + SmallVector destOperands; + destOperands.push_back(lowerBound); + auto iterOperands = forOp.getIterOperands(); + destOperands.append(iterOperands.begin(), iterOperands.end()); + rewriter.setInsertionPointToEnd(initBlock); + rewriter.create(loc, conditionBlock, destOperands); + + // With the body block done, we can fill in the condition block. + rewriter.setInsertionPointToEnd(conditionBlock); + auto comparison = rewriter.create(loc, pred, iv, upperBound); + + rewriter.create(loc, comparison, firstBodyBlock, + ArrayRef(), endBlock, + ArrayRef()); + // 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 { + + 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(ctx, /*benefit=*/2); + populateSCFToControlFlowConversionPatterns(patterns); + + // Set up conversion target + ConversionTarget target(*ctx); + target.addIllegalOp(); + target.markUnknownOpDynamicallyLegal([](Operation *) { return true; }); + + if (failed(applyPartialConversion(modOp, target, std::move(patterns)))) + signalPassFailure(); + }; +}; +} // namespace + +namespace dynamatic { +std::unique_ptr> createLowerScfToCf() { + return std::make_unique(); +} +} // namespace dynamatic diff --git a/tools/dynamatic-opt/CMakeLists.txt b/tools/dynamatic-opt/CMakeLists.txt index 7294d4894..48f6d4151 100644 --- a/tools/dynamatic-opt/CMakeLists.txt +++ b/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