From c450cd808797f58179ae2665e0ba5a64b2155ab1 Mon Sep 17 00:00:00 2001 From: shundroid Date: Tue, 14 Oct 2025 23:49:54 +0900 Subject: [PATCH 1/8] correct pass construct for HandshakeSpeculation --- .../include/experimental/Transforms/Passes.h | 5 ++++ .../include/experimental/Transforms/Passes.td | 1 - .../Speculation/HandshakeSpeculation.h | 5 ---- .../lib/Transforms/Speculation/CMakeLists.txt | 2 ++ .../Speculation/HandshakeSpeculation.cpp | 29 +++++++++---------- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/experimental/include/experimental/Transforms/Passes.h b/experimental/include/experimental/Transforms/Passes.h index 310af376d7..d312b5212e 100644 --- a/experimental/include/experimental/Transforms/Passes.h +++ b/experimental/include/experimental/Transforms/Passes.h @@ -26,6 +26,11 @@ namespace dynamatic { namespace experimental { +/// TableGen-generated header files assume all passes reside in the same +/// namespace. However, in Dynamatic, some passes are defined in separate child +/// namespaces, so we explicitly brings those into the current scope here. +using namespace speculation; + /// Generate the code for registering passes. #define GEN_PASS_REGISTRATION #include "experimental/Transforms/Passes.h.inc" diff --git a/experimental/include/experimental/Transforms/Passes.td b/experimental/include/experimental/Transforms/Passes.td index 84a436821c..829baeb455 100644 --- a/experimental/include/experimental/Transforms/Passes.td +++ b/experimental/include/experimental/Transforms/Passes.td @@ -42,7 +42,6 @@ def HandshakeSpeculation : DynamaticPass<"handshake-speculation"> { The pass needs to specify the position of the Speculation Units by means of an input JSON file. }]; - let constructor = "dynamatic::experimental::speculation::createHandshakeSpeculation()"; let options = [ Option<"jsonPath", "json-path", "std::string", "", "Path to JSON-formatted file containing the positions for speculative " diff --git a/experimental/include/experimental/Transforms/Speculation/HandshakeSpeculation.h b/experimental/include/experimental/Transforms/Speculation/HandshakeSpeculation.h index f58d982688..8046779c3b 100644 --- a/experimental/include/experimental/Transforms/Speculation/HandshakeSpeculation.h +++ b/experimental/include/experimental/Transforms/Speculation/HandshakeSpeculation.h @@ -22,12 +22,7 @@ namespace dynamatic { namespace experimental { namespace speculation { -std::unique_ptr -createHandshakeSpeculation(const std::string &jsonPath = "", - bool automatic = true); - #define GEN_PASS_DECL_HANDSHAKESPECULATION -#define GEN_PASS_DEF_HANDSHAKESPECULATION #include "experimental/Transforms/Passes.h.inc" } // namespace speculation diff --git a/experimental/lib/Transforms/Speculation/CMakeLists.txt b/experimental/lib/Transforms/Speculation/CMakeLists.txt index c113de5da9..68c75c6ac1 100644 --- a/experimental/lib/Transforms/Speculation/CMakeLists.txt +++ b/experimental/lib/Transforms/Speculation/CMakeLists.txt @@ -1,3 +1,5 @@ +include_directories(${DYNAMATIC_SOURCE_DIR}/experimental/include/experimental/Transforms/Speculation) + add_dynamatic_library(DynamaticSpeculation SpeculationPlacement.cpp HandshakeSpeculation.cpp diff --git a/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp b/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp index aa8c609e63..cd60080a75 100644 --- a/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp +++ b/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp @@ -12,7 +12,6 @@ #include "experimental/Transforms/Speculation/HandshakeSpeculation.h" #include "dynamatic/Analysis/NameAnalysis.h" -#include "dynamatic/Dialect/Handshake/HandshakeAttributes.h" #include "dynamatic/Dialect/Handshake/HandshakeInterfaces.h" #include "dynamatic/Dialect/Handshake/HandshakeOps.h" #include "dynamatic/Dialect/Handshake/HandshakeTypes.h" @@ -20,7 +19,6 @@ #include "dynamatic/Support/DynamaticPass.h" #include "experimental/Transforms/Speculation/PlacementFinder.h" #include "experimental/Transforms/Speculation/SpeculationPlacement.h" -#include "mlir/IR/Attributes.h" #include "mlir/IR/BuiltinOps.h" #include "mlir/IR/MLIRContext.h" #include "mlir/IR/OperationSupport.h" @@ -38,16 +36,24 @@ using namespace dynamatic::handshake; using namespace dynamatic::experimental; using namespace dynamatic::experimental::speculation; -namespace { +namespace dynamatic { +namespace experimental { +namespace speculation { + +// Implement the base class and auto-generated create functions. +// Must be called from the .cpp file to avoid multiple definitions +#define GEN_PASS_DEF_HANDSHAKESPECULATION +#include "experimental/Transforms/Passes.h.inc" + +} // namespace speculation +} // namespace experimental +} // namespace dynamatic struct HandshakeSpeculationPass : public dynamatic::experimental::speculation::impl:: HandshakeSpeculationBase { - HandshakeSpeculationPass(const std::string &jsonPath = "", - bool automatic = true) { - this->jsonPath = jsonPath; - this->automatic = automatic; - } + using HandshakeSpeculationBase< + HandshakeSpeculationPass>::HandshakeSpeculationBase; void runDynamaticPass() override; @@ -88,7 +94,6 @@ struct HandshakeSpeculationPass // their type requirements. LogicalResult addNonSpecOp(); }; -} // namespace // The list item to trace the branches that need to be replicated struct BranchTracingItem { @@ -876,9 +881,3 @@ void HandshakeSpeculationPass::runDynamaticPass() { if (failed(addNonSpecOp())) return signalPassFailure(); } - -std::unique_ptr -dynamatic::experimental::speculation::createHandshakeSpeculation( - const std::string &jsonPath, bool automatic) { - return std::make_unique(jsonPath, automatic); -} From 58507f842806519608c268c458ea5132c3994ca0 Mon Sep 17 00:00:00 2001 From: shundroid Date: Wed, 15 Oct 2025 12:14:12 +0900 Subject: [PATCH 2/8] latest spec v1 --- data/components.json | 35 ++-- .../include/experimental/Transforms/Passes.h | 1 + .../include/experimental/Transforms/Passes.td | 11 + .../Speculation/HandshakeSpecPostBuffer.h | 17 ++ .../lib/Transforms/Speculation/CMakeLists.txt | 1 + .../Speculation/HandshakeSpecPostBuffer.cpp | 197 ++++++++++++++++++ .../Speculation/HandshakeSpeculation.cpp | 136 ++++++------ .../handshake/speculation/spec_save_commit.py | 32 +-- .../handshake/speculation/speculator.py | 2 +- .../Dialect/Handshake/HandshakeOps.td | 65 ++++++ integration-test/fixed/buffer.json | 16 -- integration-test/if_convert/spec.json | 4 +- integration-test/loop_path/buffer.json | 37 ---- integration-test/nested_loop/buffer.json | 58 ------ integration-test/single_loop/buffer.json | 44 ---- integration-test/sparse/buffer.json | 9 - integration-test/subdiag/buffer.json | 30 --- integration-test/subdiag_fast/buffer.json | 23 -- lib/Support/CFG.cpp | 2 +- .../BufferPlacement/BufferPlacementMILP.cpp | 18 ++ tools/hls-verifier/hls-verifier.cpp | 2 +- tools/integration/run_spec_integration.py | 140 ++++++++----- 22 files changed, 498 insertions(+), 382 deletions(-) create mode 100644 experimental/include/experimental/Transforms/Speculation/HandshakeSpecPostBuffer.h create mode 100644 experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp delete mode 100644 integration-test/fixed/buffer.json delete mode 100644 integration-test/loop_path/buffer.json delete mode 100644 integration-test/nested_loop/buffer.json delete mode 100644 integration-test/single_loop/buffer.json delete mode 100644 integration-test/sparse/buffer.json delete mode 100644 integration-test/subdiag/buffer.json delete mode 100644 integration-test/subdiag_fast/buffer.json diff --git a/data/components.json b/data/components.json index 7a3716502d..3f9a76fcb5 100644 --- a/data/components.json +++ b/data/components.json @@ -445,14 +445,10 @@ "handshake.addf.flopoco": { "latency": { "64": { - "2.705000": 12.0, - "5.091333": 7.0, - "9.068000": 2.0 + "2.705000": 12.0 }, "32": { - "2.798000": 36.0, - "2.922000": 10.0, - "3.649333": 6.0 + "3.649333": 9.0 } }, "delay": { @@ -511,12 +507,11 @@ "handshake.addf.vivado": { "latency": { "64": { - "1.0" : 9.0, - "2.0" : 9.0, - "3.75" : 9.0, - "4.1" : 9.0, - "5.0" : 9.0 - } + "2.705000": 12.0 + }, + "32": { + "3.649333": 9.0 + } }, "delay": { "data": { @@ -574,7 +569,6 @@ "handshake.subf.flopoco": { "latency": { "64": { - "1.0": 11.0, "2.0": 9.0 } }, @@ -634,7 +628,6 @@ "handshake.subf.vivado": { "latency": { "64": { - "1.0": 11.0, "2.0": 9.0 } }, @@ -694,14 +687,10 @@ "handshake.mulf.flopoco": { "latency": { "32": { - "2.034000": 26.0, - "2.783000": 4.0, - "2.875333": 2.0 + "2.783000": 4.0 }, "64": { - "2.046000": 56.0, - "2.758000": 6.0, - "4.242333": 3.0 + "4.242333": 4.0 } }, "delay": { @@ -759,9 +748,11 @@ }, "handshake.mulf.vivado": { "latency": { + "32": { + "2.783000": 4.0 + }, "64": { - "1.0" : 4.0, - "2.0" : 4.0 + "4.242333": 4.0 } }, "delay": { diff --git a/experimental/include/experimental/Transforms/Passes.h b/experimental/include/experimental/Transforms/Passes.h index d312b5212e..abc21212d8 100644 --- a/experimental/include/experimental/Transforms/Passes.h +++ b/experimental/include/experimental/Transforms/Passes.h @@ -20,6 +20,7 @@ #include "experimental/Transforms/LSQSizing/HandshakeSizeLSQs.h" #include "experimental/Transforms/ResourceSharing/Crush.h" #include "experimental/Transforms/Rigidification/HandshakeRigidification.h" +#include "experimental/Transforms/Speculation/HandshakeSpecPostBuffer.h" #include "experimental/Transforms/Speculation/HandshakeSpeculation.h" #include "mlir/Pass/Pass.h" diff --git a/experimental/include/experimental/Transforms/Passes.td b/experimental/include/experimental/Transforms/Passes.td index 829baeb455..9274e6e73e 100644 --- a/experimental/include/experimental/Transforms/Passes.td +++ b/experimental/include/experimental/Transforms/Passes.td @@ -52,6 +52,17 @@ def HandshakeSpeculation : DynamaticPass<"handshake-speculation"> { "specified in the JSON-formatted file.">]; } +def HandshakeSpecPostBuffer : DynamaticPass<"handshake-spec-post-buffer"> { + let summary = "Post-buffering speculation pass"; + let description = [{ + Speculation integration requires some steps to be performed after the + buffer placement pass: + - Finalize speculative units + }]; + let options = [ + ]; +} + def HandshakePlaceBuffersCustom : DynamaticPass<"handshake-placebuffers-custom"> { let summary = "Place buffers on specific channels"; let description = [{ Placing a single buffer on a specific output channel of diff --git a/experimental/include/experimental/Transforms/Speculation/HandshakeSpecPostBuffer.h b/experimental/include/experimental/Transforms/Speculation/HandshakeSpecPostBuffer.h new file mode 100644 index 0000000000..83fd4a71ec --- /dev/null +++ b/experimental/include/experimental/Transforms/Speculation/HandshakeSpecPostBuffer.h @@ -0,0 +1,17 @@ +#ifndef DYNAMATIC_TRANSFORMS_HANDSHAKE_SPEC_POST_BUFFER_H +#define DYNAMATIC_TRANSFORMS_HANDSHAKE_SPEC_POST_BUFFER_H + +#include "dynamatic/Support/DynamaticPass.h" + +namespace dynamatic { +namespace experimental { +namespace speculation { + +#define GEN_PASS_DECL_HANDSHAKESPECPOSTBUFFER +#include "experimental/Transforms/Passes.h.inc" + +} // namespace speculation +} // namespace experimental +} // namespace dynamatic + +#endif // DYNAMATIC_TRANSFORMS_HANDSHAKE_SPEC_POST_BUFFER_H diff --git a/experimental/lib/Transforms/Speculation/CMakeLists.txt b/experimental/lib/Transforms/Speculation/CMakeLists.txt index 68c75c6ac1..fa5bb59409 100644 --- a/experimental/lib/Transforms/Speculation/CMakeLists.txt +++ b/experimental/lib/Transforms/Speculation/CMakeLists.txt @@ -3,6 +3,7 @@ include_directories(${DYNAMATIC_SOURCE_DIR}/experimental/include/experimental/Tr add_dynamatic_library(DynamaticSpeculation SpeculationPlacement.cpp HandshakeSpeculation.cpp + HandshakeSpecPostBuffer.cpp PlacementFinder.cpp DEPENDS diff --git a/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp b/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp new file mode 100644 index 0000000000..948c4826c5 --- /dev/null +++ b/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp @@ -0,0 +1,197 @@ +#include "HandshakeSpecPostBuffer.h" +#include "dynamatic/Dialect/Handshake/HandshakeInterfaces.h" +#include "dynamatic/Dialect/Handshake/HandshakeOps.h" +#include "dynamatic/Support/CFG.h" +#include "dynamatic/Support/LLVM.h" +#include "mlir/IR/AsmState.h" +#include "mlir/IR/Builders.h" +#include "mlir/IR/Diagnostics.h" +#include "mlir/IR/OperationSupport.h" +#include "mlir/IR/Value.h" +#include "mlir/Support/LLVM.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/Support/ErrorHandling.h" + +using namespace llvm::sys; +using namespace mlir; +using namespace dynamatic; +using namespace dynamatic::handshake; +using namespace dynamatic::experimental; +using namespace dynamatic::experimental::speculation; + +namespace dynamatic { +namespace experimental { +namespace speculation { + +// Implement the base class and auto-generated create functions. +// Must be called from the .cpp file to avoid multiple definitions +#define GEN_PASS_DEF_HANDSHAKESPECPOSTBUFFER +#include "experimental/Transforms/Passes.h.inc" + +} // namespace speculation +} // namespace experimental +} // namespace dynamatic + +struct HandshakeSpecPostBufferPass + : public dynamatic::experimental::speculation::impl:: + HandshakeSpecPostBufferBase { + using HandshakeSpecPostBufferBase< + HandshakeSpecPostBufferPass>::HandshakeSpecPostBufferBase; + void runDynamaticPass() override; +}; + +static Operation *getUserSkippingBuffers(Value val) { + Operation *uniqueUser = *val.getUsers().begin(); + if (auto bufOp = dyn_cast(uniqueUser)) { + return getUserSkippingBuffers(bufOp.getResult()); + } + return uniqueUser; +} + +static handshake::ConditionalBranchOp findControlBranch(FuncOp funcOp, + unsigned bb) { + for (auto condBrOp : funcOp.getOps()) { + if (auto brBB = getLogicBB(condBrOp); !brBB || brBB != bb) + continue; + + llvm::errs() << "Candidate: "; + condBrOp->dump(); + + for (Value result : condBrOp->getResults()) { + for (Operation *user : result.getUsers()) { + + if (isBackedge(result, user)) + return condBrOp; + llvm::errs() << "Rejecting user: "; + user->dump(); + } + } + } + + return nullptr; +} + +void HandshakeSpecPostBufferPass::runDynamaticPass() { + ModuleOp modOp = getOperation(); + + // Support only one funcOp + assert(std::distance(modOp.getOps().begin(), + modOp.getOps().end()) == 1 && + "Expected a single FuncOp in the module"); + + FuncOp funcOp = *modOp.getOps().begin(); + + SpecPreBufferOp1 specOp1 = *funcOp.getOps().begin(); + SpecPreBufferOp2 specOp2 = *funcOp.getOps().begin(); + + unsigned specBB = getLogicBB(specOp1).value(); + + OpBuilder builder(&getContext()); + builder.setInsertionPoint(specOp1); + + SpeculatorOp speculator = builder.create( + specOp1.getLoc(), specOp1.getDataOut().getType(), specOp2.getDataIn(), + specOp1.getTrigger(), specOp1.getFifoDepth()); + setBB(speculator, specBB); + + // speculator.setConstant(constant); + // speculator.setDefaultValue(defaultValue); + + specOp1.getDataOut().replaceAllUsesWith(speculator.getDataOut()); + specOp2.getSaveCtrl().replaceAllUsesWith(speculator.getSaveCtrl()); + specOp2.getCommitCtrl().replaceAllUsesWith(speculator.getCommitCtrl()); + specOp2.getSCIsMisspec().replaceAllUsesWith(speculator.getSCIsMisspec()); + + // Construct Save Commit Control + auto branchDiscardCondNonMisspec = cast( + getUserSkippingBuffers(speculator.getSCIsMisspec())); + + // This branch will propagate the signal SCCommitControl according to + // the control branch condition, which comes from branchDiscardCondNonMisSpec + auto branchReplicated = builder.create( + branchDiscardCondNonMisspec.getLoc(), + branchDiscardCondNonMisspec.getTrueResult(), + speculator.getSCCommitCtrl()); + setBB(branchReplicated, specBB); + + // We create a Merge operation to join SCCSaveCtrl and SCCommitCtrl signals + SmallVector mergeOperands; + mergeOperands.push_back(speculator.getSCSaveCtrl()); + + ConditionalBranchOp controlBranch = findControlBranch(funcOp, specBB); + if (controlBranch == nullptr) { + specOp1->emitError() << "Could not find backedge within speculation bb: " + << specBB << ".\n"; + return signalPassFailure(); + } + + // Helper function to check if a value leads to a Backedge + auto isBranchBackedge = [&](Value result) { + return llvm::any_of(result.getUsers(), [&](Operation *user) { + return isBackedge(result, user); + }); + }; + + // We need to send the control token to the same path that the speculative + // token followed. Hence, if any branch output leads to a backedge, replicate + // the branch in the SaveCommit control path. + + // Check if trueResult of controlBranch leads to a backedge (loop) + if (isBranchBackedge(controlBranch.getTrueResult())) { + mergeOperands.push_back(branchReplicated.getTrueResult()); + } + // Check if falseResult of controlBranch leads to a backedge (loop) + else if (isBranchBackedge(controlBranch.getFalseResult())) { + mergeOperands.push_back(branchReplicated.getFalseResult()); + } + // If neither trueResult nor falseResult leads to a backedge, handle the error + else { + controlBranch->emitError() + << "Could not find the backedge in the Control Branch " << specBB + << "\n"; + return signalPassFailure(); + } + + // All the inputs to the merge operation are ready + auto mergeOp = builder.create(branchReplicated.getLoc(), + mergeOperands); + setBB(mergeOp, specBB); + + specOp1.getSCSaveCtrl().replaceAllUsesWith(mergeOp.getResult()); + + specOp1->erase(); + specOp2->erase(); + + for (auto commitOp : funcOp.getOps()) { + builder.setInsertionPoint(commitOp); + // To maintain high throughput: commit op *sometimes* joins a control with + // data from the iteration i-1. We need a 1-slot buffer to hold the control + // signal + auto bufOp1 = + builder.create(builder.getUnknownLoc(), commitOp.getCtrl(), 1, + BufferType::FIFO_BREAK_NONE); + inheritBB(commitOp, bufOp1); + bufOp1.getOperand().replaceAllUsesExcept(bufOp1.getResult(), bufOp1); + + // To avoid deadlock: On misspeculation, kill follows resend. When "resend" + // is sent, misspeculated data doesn't join with "kill" signal. The stall of + // this data possibly leads to a deadlock ("resend" is never accepted by a + // save-commit). Therefore, we need a 1-slot buffer to store the token + auto bufOp2 = + builder.create(builder.getUnknownLoc(), commitOp.getDataIn(), + 1, BufferType::FIFO_BREAK_NONE); + inheritBB(commitOp, bufOp2); + bufOp2.getOperand().replaceAllUsesExcept(bufOp2.getResult(), bufOp2); + } + + for (auto specBranchOp : funcOp.getOps()) { + Value specResult = specBranchOp.getTrueResult(); + builder.setInsertionPointAfterValue(specResult); + // Create a buffer to hold the delayed operand + auto bufOp = builder.create(builder.getUnknownLoc(), specResult, + 1, BufferType::FIFO_BREAK_NONE); + inheritBBFromValue(specResult, bufOp); + specResult.replaceAllUsesExcept(bufOp.getResult(), bufOp); + } +} diff --git a/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp b/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp index cd60080a75..f54ffc3bbf 100644 --- a/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp +++ b/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp @@ -59,7 +59,8 @@ struct HandshakeSpeculationPass private: SpeculationPlacements placements; - SpeculatorOp specOp; + SpecPreBufferOp1 specOp1; + SpecPreBufferOp2 specOp2; // In the placeCommits method, commit units are temporarily connected to // this value as an alternative to control signals and are subsequently @@ -188,7 +189,7 @@ static std::optional findExistingBranch(Value condition, // branches it finds in the way. It stops at commits and connects them to the // newly created path with value ctrlSignal static void -routeCommitControlRecursive(MLIRContext *ctx, SpeculatorOp &specOp, +routeCommitControlRecursive(MLIRContext *ctx, SpecPreBufferOp2 &specOp, llvm::DenseSet &arrived, OpOperand &currOpOperand, std::vector &branchTrace) { @@ -240,7 +241,7 @@ routeCommitControlRecursive(MLIRContext *ctx, SpeculatorOp &specOp, /*trueResultType=*/conditionOperand.getType(), /*falseResultType=*/conditionOperand.getType(), /*specTag=*/valueForSpecTag, conditionOperand); - inheritBB(specOp, *branchDiscardNonSpec); + inheritBB(valueForSpecTag.getDefiningOp(), *branchDiscardNonSpec); } std::optional branchReplicated = @@ -253,7 +254,7 @@ routeCommitControlRecursive(MLIRContext *ctx, SpeculatorOp &specOp, branchDiscardNonSpec->getLoc(), /*condition=*/branchDiscardNonSpec->getTrueResult(), /*data=*/ctrlSignal); - inheritBB(specOp, *branchReplicated); + inheritBB(*branchDiscardNonSpec, *branchReplicated); } // Update ctrlSignal @@ -313,17 +314,17 @@ LogicalResult HandshakeSpeculationPass::routeCommitControl() { llvm::DenseSet arrived; std::vector branchTrace; // Start traversal from the speculator - for (OpOperand &succOpOperand : specOp.getDataOut().getUses()) { - routeCommitControlRecursive(&getContext(), specOp, arrived, succOpOperand, + for (OpOperand &succOpOperand : specOp1.getDataOut().getUses()) { + routeCommitControlRecursive(&getContext(), specOp2, arrived, succOpOperand, branchTrace); } // Start traversal from save-commit units for (auto saveCommitOp : - mlir::cast(specOp->getParentOp()).getOps()) { + mlir::cast(specOp1->getParentOp()).getOps()) { for (OpOperand &succOpOperand : saveCommitOp.getDataOut().getUses()) { branchTrace.clear(); - routeCommitControlRecursive(&getContext(), specOp, arrived, succOpOperand, - branchTrace); + routeCommitControlRecursive(&getContext(), specOp2, arrived, + succOpOperand, branchTrace); } } @@ -333,7 +334,7 @@ LogicalResult HandshakeSpeculationPass::routeCommitControl() { LogicalResult HandshakeSpeculationPass::placeCommits() { // Create a temporal value to connect the commits - Value commitCtrl = specOp.getCommitCtrl(); + Value commitCtrl = specOp2.getCommitCtrl(); OpBuilder builder(&getContext()); // Build a temporary control value using mlir::UnrealizedConversionCastOp @@ -344,7 +345,7 @@ LogicalResult HandshakeSpeculationPass::placeCommits() { fakeControlForCommits = builder .create( - specOp->getLoc(), commitCtrl.getType(), ValueRange{}) + specOp2->getLoc(), commitCtrl.getType(), ValueRange{}) .getResult(0); // Place commits and connect to the fake control signal @@ -359,7 +360,7 @@ LogicalResult HandshakeSpeculationPass::placeCommits() { SpecCommitOp newOp = builder.create( dstOp->getLoc(), /*resultType=*/srcOpResult.getType(), /*dataIn=*/srcOpResult, /*ctrl=*/fakeControlForCommits.value()); - inheritBB(dstOp, newOp); + inheritBB(srcOpResult.getDefiningOp(), newOp); // Connect the new CommitOp to dstOp operand->set(newOp.getResult()); @@ -399,12 +400,8 @@ LogicalResult HandshakeSpeculationPass::placeSaveCommits(Value ctrlSignal) { return success(); } -static handshake::ConditionalBranchOp findControlBranch(Operation *op) { - handshake::FuncOp funcOp = op->getParentOfType(); - assert(funcOp && "op should have parent function"); - auto handshakeBlocks = getLogicBBs(funcOp); - unsigned bb = getLogicBB(op).value(); - +static handshake::ConditionalBranchOp findControlBranch(FuncOp funcOp, + unsigned bb) { for (auto condBrOp : funcOp.getOps()) { if (auto brBB = getLogicBB(condBrOp); !brBB || brBB != bb) continue; @@ -427,9 +424,13 @@ FailureOr HandshakeSpeculationPass::generateSaveCommitCtrl() { // The save commits are a result of a control branch being in the BB // The control path for the SC needs to replicate the branch - ConditionalBranchOp controlBranch = findControlBranch(specOp); + handshake::FuncOp funcOp = specOp1->getParentOfType(); + auto handshakeBlocks = getLogicBBs(funcOp); + unsigned bb = getLogicBB(specOp1).value(); + + ConditionalBranchOp controlBranch = findControlBranch(funcOp, bb); if (controlBranch == nullptr) { - specOp->emitError() << "Could not find backedge within speculation bb.\n"; + specOp1->emitError() << "Could not find backedge within speculation bb.\n"; return failure(); } @@ -438,7 +439,7 @@ FailureOr HandshakeSpeculationPass::generateSaveCommitCtrl() { // The tokens take differents paths. One (SCSaveCtrl) needs to always reach // the SC, the other (SCCommitCtrl) should follow the actual branches // similarly to the Commits - builder.setInsertionPointAfterValue(specOp.getSCCommitCtrl()); + builder.setInsertionPointAfterValue(specOp2.getSCIsMisspec()); // First, discard if speculation didn't happen @@ -450,63 +451,23 @@ FailureOr HandshakeSpeculationPass::generateSaveCommitCtrl() { controlBranch.getLoc(), /*trueResultType=*/conditionOperand.getType(), /*falseResultType=*/conditionOperand.getType(), - /*specTag=*/specOp.getDataOut(), conditionOperand); - inheritBB(specOp, branchDiscardCondNonSpec); + /*specTag=*/conditionOperand, conditionOperand); + inheritBB(specOp1, branchDiscardCondNonSpec); + branchDiscardCondNonSpec->setAttr("specv1_branchDiscardCondNonSpec", + builder.getUnitAttr()); // Second, discard if speculation happened but it was correct // Create a conditional branch driven by SCBranchControl from speculator // SCBranchControl discards the commit-like signal when speculation is correct auto branchDiscardCondNonMisspec = builder.create( - branchDiscardCondNonSpec.getLoc(), specOp.getSCIsMisspec(), + branchDiscardCondNonSpec.getLoc(), specOp2.getSCIsMisspec(), branchDiscardCondNonSpec.getTrueResult()); - inheritBB(specOp, branchDiscardCondNonMisspec); - - // This branch will propagate the signal SCCommitControl according to - // the control branch condition, which comes from branchDiscardCondNonMisSpec - auto branchReplicated = builder.create( - branchDiscardCondNonMisspec.getLoc(), - branchDiscardCondNonMisspec.getTrueResult(), specOp.getSCCommitCtrl()); - inheritBB(specOp, branchReplicated); - - // We create a Merge operation to join SCCSaveCtrl and SCCommitCtrl signals - SmallVector mergeOperands; - mergeOperands.push_back(specOp.getSCSaveCtrl()); - - // Helper function to check if a value leads to a Backedge - auto isBranchBackedge = [&](Value result) { - return llvm::any_of(result.getUsers(), [&](Operation *user) { - return isBackedge(result, user); - }); - }; - - // We need to send the control token to the same path that the speculative - // token followed. Hence, if any branch output leads to a backedge, replicate - // the branch in the SaveCommit control path. - - // Check if trueResult of controlBranch leads to a backedge (loop) - if (isBranchBackedge(controlBranch.getTrueResult())) { - mergeOperands.push_back(branchReplicated.getTrueResult()); - } - // Check if falseResult of controlBranch leads to a backedge (loop) - else if (isBranchBackedge(controlBranch.getFalseResult())) { - mergeOperands.push_back(branchReplicated.getFalseResult()); - } - // If neither trueResult nor falseResult leads to a backedge, handle the error - else { - unsigned bb = getLogicBB(specOp).value(); - controlBranch->emitError() - << "Could not find the backedge in the Control Branch " << bb << "\n"; - return failure(); - } - - // All the inputs to the merge operation are ready - auto mergeOp = builder.create(branchReplicated.getLoc(), - mergeOperands); - inheritBB(specOp, mergeOp); + inheritBB(specOp2, branchDiscardCondNonMisspec); - // The control signal is the result of the merge op. - return mergeOp.getResult(); + // Tentatively use specOp1.getSCSaveCtrl for the control signal for + // save-commits. Post-buffering pass replaces this. + return specOp1.getSCSaveCtrl(); } std::optional findControlInputToBB(handshake::FuncOp &funcOp, @@ -584,17 +545,20 @@ LogicalResult HandshakeSpeculationPass::placeSpeculator() { // resultType is tentative and will be updated in the addSpecTag algorithm // later. - specOp = builder.create( + specOp1 = builder.create( dstOp->getLoc(), /*resultType=*/srcOpResult.getType(), - /*dataIn=*/srcOpResult, /*specIn=*/specTrigger.value(), fifoDepth); + /*specIn=*/specTrigger.value(), fifoDepth); + specOp2 = builder.create(dstOp->getLoc(), + /*dataIn=*/srcOpResult, fifoDepth); // Replace uses of the original source operation's result with the // speculator's result, except in the speculator's operands (otherwise this // would create a self-loop from the speculator to the speculator) - srcOpResult.replaceAllUsesExcept(specOp.getDataOut(), specOp); + srcOpResult.replaceAllUsesExcept(specOp1.getDataOut(), specOp2); // Assign a Basic Block to the speculator - inheritBB(dstOp, specOp); + inheritBB(dstOp, specOp1); + inheritBB(dstOp, specOp2); return success(); } @@ -774,12 +738,13 @@ addSpecTagToSpecRegionRecursive(MLIRContext &ctx, OpOperand &opOperand, LogicalResult HandshakeSpeculationPass::addSpecTagToSpecRegion() { llvm::DenseSet visited; - visited.insert(specOp); + visited.insert(specOp1); + visited.insert(specOp2); // For the speculator, perform downstream traversal to only dataOut, skipping // control signals. The upstream dataIn will be handled by the recursive // traversal. - for (OpOperand &opOperand : specOp.getDataOut().getUses()) { + for (OpOperand &opOperand : specOp1.getDataOut().getUses()) { if (failed(addSpecTagToSpecRegionRecursive(getContext(), opOperand, true, visited))) return failure(); @@ -788,7 +753,7 @@ LogicalResult HandshakeSpeculationPass::addSpecTagToSpecRegion() { } LogicalResult HandshakeSpeculationPass::addNonSpecOp() { - auto funcOp = cast(specOp->getParentOp()); + auto funcOp = cast(specOp1->getParentOp()); OpBuilder builder(&getContext()); for (auto mergeLikeOp : funcOp.getOps()) { @@ -880,4 +845,21 @@ void HandshakeSpeculationPass::runDynamaticPass() { // to satisfy their type requirements. if (failed(addNonSpecOp())) return signalPassFailure(); + + // quick fix + handshake::FuncOp funcOp = specOp1->getParentOfType(); + for (auto branch : funcOp.getOps()) { + if (branch->getAttr("specv1_branchDiscardCondNonSpec")) { + unsigned bb = getLogicBB(specOp1).value(); + ConditionalBranchOp controlBranch = findControlBranch(funcOp, bb); + if (controlBranch == nullptr) { + specOp1->emitError() + << "Could not find backedge within speculation bb.\n"; + return signalPassFailure(); + } + auto conditionOperand = controlBranch.getConditionOperand(); + branch->setOperand(0, conditionOperand); + branch->setOperand(1, conditionOperand); + } + } } diff --git a/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/spec_save_commit.py b/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/spec_save_commit.py index b9b3f2169e..5cb5f59e17 100644 --- a/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/spec_save_commit.py +++ b/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/spec_save_commit.py @@ -148,17 +148,25 @@ def _generate_spec_save_commit(name, bitwidth, fifo_depth): outs_spec <= "0"; elsif ctrl_valid = '1' and ctrl = "100" then -- NO_CMP - -- TODO: When Empty = '1', input data should be bypassed, - -- just like when PASS or PASS_KILL, for better performance. -- Head = Curr is assumed from the specification. - -- `not Empty` ensures Curr < Tail. - CurrEn <= outs_ready and not Empty; - HeadEn <= outs_ready and not Empty; + if Empty = '1' then + CurrEn <= outs_ready and ins_valid and not Full; + HeadEn <= outs_ready and ins_valid and not Full; - ctrl_ready <= outs_ready and not Empty; - outs_valid <= not Empty; - {data("outs <= Memory(Head);", bitwidth)} - outs_spec <= "0"; + ctrl_ready <= outs_ready and ins_valid and not Full; + outs_valid <= ins_valid and not Full; + {data("outs <= ins;", bitwidth)} + outs_spec <= "0"; + else + -- `Empty = '0'` ensures Curr < Tail. + CurrEn <= outs_ready; + HeadEn <= outs_ready; + + ctrl_ready <= outs_ready; + outs_valid <= '1'; + {data("outs <= Memory(Head);", bitwidth)} + outs_spec <= "0"; + end if; end if; end process; @@ -245,7 +253,7 @@ def _generate_spec_save_commit(name, bitwidth, fifo_depth): -- if only filling but not emptying if (TailEn = '1') and (HeadEn = '0') then -- if new tail index will reach head index - if ((Tail +1) mod {fifo_depth} = Head) then + if ((Tail + 2) mod {fifo_depth} = Head) then Full <= '1'; end if; elsif (TailEn = '0') and (HeadEn = '1') then @@ -269,7 +277,7 @@ def _generate_spec_save_commit(name, bitwidth, fifo_depth): -- if only emptying but not filling if (TailEn = '0') and (HeadEn = '1') then -- if new head index will reach tail index - if ((Head +1) mod {fifo_depth} = Tail) then + if ((Head + 1) mod {fifo_depth} = Tail) then Empty <= '1'; end if; elsif (TailEn = '1') and (HeadEn = '0') then @@ -293,7 +301,7 @@ def _generate_spec_save_commit(name, bitwidth, fifo_depth): -- if only emptying but not filling if (TailEn = '0') and (CurrEn = '1') then -- if new head index will reach tail index - if ((Curr +1) mod {fifo_depth} = Tail) then + if ((Curr + 1) mod {fifo_depth} = Tail) then CurrEmpty <= '1'; end if; elsif (TailEn = '1') and (CurrEn = '0') then diff --git a/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/speculator.py b/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/speculator.py index 1c24be9a83..7aba46163a 100644 --- a/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/speculator.py +++ b/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/speculator.py @@ -714,7 +714,7 @@ def _generate_predFifo(name, bitwidth, fifo_depth): -- if only filling but not emptying if (TailEn = '1') and (HeadEn = '0') then -- if new tail index will reach head index - if ((Tail +1) mod {fifo_depth} = Head) then + if ((Tail +2) mod {fifo_depth} = Head) then Full <= '1'; end if; elsif (TailEn = '0') and (HeadEn = '1') then diff --git a/include/dynamatic/Dialect/Handshake/HandshakeOps.td b/include/dynamatic/Dialect/Handshake/HandshakeOps.td index 2f03f1bfd4..24ac22bdc3 100644 --- a/include/dynamatic/Dialect/Handshake/HandshakeOps.td +++ b/include/dynamatic/Dialect/Handshake/HandshakeOps.td @@ -1139,6 +1139,71 @@ def SpeculatorOp : Handshake_Op<"speculator", [ }]; } +def SpecPreBufferOp1 : Handshake_Op<"spec_prebuffer1", [ + HasValidSpecTag<"dataOut">, + IsSimpleHandshake<"SCSaveCtrl">, + IsIntSizedChannel<3, "SCSaveCtrl">, +]> { + let summary = ""; + let description = [{ + }]; + + let arguments = (ins ControlType:$trigger, + UI32Attr:$fifoDepth); + let results = (outs ChannelType:$dataOut, + ChannelType:$SCSaveCtrl); + + let assemblyFormat = [{ + `[` $trigger `]` attr-dict `:` type($trigger) `,` + type($dataOut) `,` type($SCSaveCtrl) + }]; + + // Infer the type of the control signals + let builders = [OpBuilder<(ins "Type":$dataOutType, "Value":$trigger, "unsigned":$fifoDepth), [{ + $_state.addOperands({trigger}); + $_state.addAttribute("fifoDepth", $_builder.getUI32IntegerAttr(fifoDepth)); + + ChannelType wideControlType = ChannelType::get($_builder.getIntegerType(3)); + + $_state.addTypes({dataOutType, wideControlType}); + }]>]; +} + +def SpecPreBufferOp2 : Handshake_Op<"spec_prebuffer2", [ + HasValidSpecTag<"dataIn">, + IsSimpleHandshake<"saveCtrl">, + IsIntSizedChannel<1, "saveCtrl">, + IsSimpleHandshake<"commitCtrl">, + IsIntSizedChannel<1, "commitCtrl">, + IsSimpleHandshake<"SCIsMisspec">, + IsIntSizedChannel<1, "SCIsMisspec"> +]> { + let summary = ""; + let description = [{ + }]; + + let arguments = (ins ChannelType:$dataIn, + UI32Attr:$fifoDepth); + let results = (outs ChannelType:$saveCtrl, ChannelType:$commitCtrl, + ChannelType:$SCIsMisspec); + + let assemblyFormat = [{ + $dataIn attr-dict `:` type($dataIn) `,` + type($saveCtrl) `,` type($commitCtrl) `,` + type($SCIsMisspec) + }]; + + // Infer the type of the control signals + let builders = [OpBuilder<(ins "Value":$dataIn, "unsigned":$fifoDepth), [{ + $_state.addOperands({dataIn}); + $_state.addAttribute("fifoDepth", $_builder.getUI32IntegerAttr(fifoDepth)); + + ChannelType ctrlType = ChannelType::get($_builder.getIntegerType(1)); + + $_state.addTypes({ctrlType, ctrlType, ctrlType}); + }]>]; +} + def SpecSaveOp : Handshake_Op<"spec_save", [ AllDataTypesMatch<["dataIn", "dataOut"]>, AllExtraSignalsMatchExcept<"spec", ["dataIn", "dataOut"]>, diff --git a/integration-test/fixed/buffer.json b/integration-test/fixed/buffer.json deleted file mode 100644 index 159aedab29..0000000000 --- a/integration-test/fixed/buffer.json +++ /dev/null @@ -1,16 +0,0 @@ -[ - { - "pred": "speculating_branch0", - "outid": 0, - "slots": 3, - "type": "fifo_break_none", - "comment": "To avoid deadlock" - }, - { - "pred": "fork6", - "outid": 4, - "slots": 2, - "type": "fifo_break_none", - "comment": "To achieve better II" - } -] diff --git a/integration-test/if_convert/spec.json b/integration-test/if_convert/spec.json index a808593393..ec0933582e 100644 --- a/integration-test/if_convert/spec.json +++ b/integration-test/if_convert/spec.json @@ -2,7 +2,7 @@ "speculator": { "operation-name": "select0", "operand-idx": 0, - "fifo-depth": 7 + "fifo-depth": 6 }, - "save-commits-fifo-depth": 7 + "save-commits-fifo-depth": 6 } diff --git a/integration-test/loop_path/buffer.json b/integration-test/loop_path/buffer.json deleted file mode 100644 index 93a1865830..0000000000 --- a/integration-test/loop_path/buffer.json +++ /dev/null @@ -1,37 +0,0 @@ -[ - { - "pred": "speculating_branch0", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "To avoid deadlock" - }, - { - "pred": "cmpi2", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "To achieve better II" - }, - { - "pred": "fork3", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "To achieve better II" - }, - { - "pred": "spec_commit4", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "Buffer non-spec token to prevent II=2 locking" - }, - { - "pred": "fork11", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "To absorb latency for spec_commit4 (ctrl)" - } -] diff --git a/integration-test/nested_loop/buffer.json b/integration-test/nested_loop/buffer.json deleted file mode 100644 index c96f590b31..0000000000 --- a/integration-test/nested_loop/buffer.json +++ /dev/null @@ -1,58 +0,0 @@ -[ - { - "pred": "speculating_branch1", - "outid": 0, - "slots": 5, - "type": "fifo_break_none", - "comment": "To avoid deadlock" - }, - { - "pred": "fork18", - "outid": 0, - "slots": 4, - "type": "fifo_break_none", - "comment": "To absorb the initial latency (sc ctrl)" - }, - { - "pred": "fork14", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "To absorb latency for spec_commit8 (ctrl)" - }, - { - "pred": "spec_commit9", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "Buffer non-spec token to prevent II=2 locking" - }, - { - "pred": "spec_commit8", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "Buffer non-spec token to prevent II=2 locking" - }, - { - "pred": "extsi3", - "outid": 0, - "slots": 4, - "type": "fifo_break_none", - "comment": "To absorb latency for spec_commit1 (data)" - }, - { - "pred": "addi0", - "outid": 0, - "slots": 4, - "type": "fifo_break_none", - "comment": "To absorb latency for spec_commit9 (data)" - }, - { - "pred": "cmpi0", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "To prevent II=2 locking" - } -] diff --git a/integration-test/single_loop/buffer.json b/integration-test/single_loop/buffer.json deleted file mode 100644 index e61dfa731e..0000000000 --- a/integration-test/single_loop/buffer.json +++ /dev/null @@ -1,44 +0,0 @@ -[ - { - "pred": "speculating_branch0", - "outid": 0, - "slots": 5, - "type": "fifo_break_none", - "comment": "To avoid deadlock" - }, - { - "pred": "trunci0", - "outid": 0, - "slots": 4, - "type": "fifo_break_none", - "comment": "To absorb latency for spec_commit4 (data)" - }, - { - "pred": "extsi1", - "outid": 0, - "slots": 5, - "type": "fifo_break_none", - "comment": "To absorb latency for spec_commit1 (data)" - }, - { - "pred": "fork9", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "To absorb latency for spec_commit5 (ctrl)" - }, - { - "pred": "spec_commit4", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "Buffer non-spec token to prevent II=2 locking" - }, - { - "pred": "spec_commit5", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "Buffer non-spec token to prevent II=2 locking" - } -] diff --git a/integration-test/sparse/buffer.json b/integration-test/sparse/buffer.json deleted file mode 100644 index 601f7ad9ef..0000000000 --- a/integration-test/sparse/buffer.json +++ /dev/null @@ -1,9 +0,0 @@ -[ - { - "pred": "speculating_branch0", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "To avoid deadlock" - } -] diff --git a/integration-test/subdiag/buffer.json b/integration-test/subdiag/buffer.json deleted file mode 100644 index be772be938..0000000000 --- a/integration-test/subdiag/buffer.json +++ /dev/null @@ -1,30 +0,0 @@ -[ - { - "pred": "speculating_branch0", - "outid": 0, - "slots": 8, - "type": "fifo_break_none", - "comment": "To avoid deadlock" - }, - { - "pred": "cmpi1", - "outid": 0, - "slots": 8, - "type": "fifo_break_none", - "comment": "To achieve better II" - }, - { - "pred": "load2", - "outid": 1, - "slots": 7, - "type": "fifo_break_none", - "comment": "To achieve better II" - }, - { - "pred": "trunci2", - "outid": 0, - "slots": 2, - "type": "fifo_break_none", - "comment": "To achieve better II" - } -] diff --git a/integration-test/subdiag_fast/buffer.json b/integration-test/subdiag_fast/buffer.json deleted file mode 100644 index 268c238228..0000000000 --- a/integration-test/subdiag_fast/buffer.json +++ /dev/null @@ -1,23 +0,0 @@ -[ - { - "pred": "speculating_branch0", - "outid": 0, - "slots": 15, - "type": "fifo_break_none", - "comment": "To avoid deadlock" - }, - { - "pred": "cmpi1", - "outid": 0, - "slots": 15, - "type": "fifo_break_none", - "comment": "To achieve better II" - }, - { - "pred": "load2", - "outid": 1, - "slots": 13, - "type": "fifo_break_none", - "comment": "To achieve better II" - } -] diff --git a/lib/Support/CFG.cpp b/lib/Support/CFG.cpp index 718eca63f0..1c70dff30f 100644 --- a/lib/Support/CFG.cpp +++ b/lib/Support/CFG.cpp @@ -153,7 +153,7 @@ static bool followToBlock(Operation *op, unsigned &bb, /// outside blocks during backedge identification. static inline bool canGoThroughOutsideBlocks(Operation *op) { return isa(op); + handshake::TruncIOp, handshake::BufferOp>(op); } /// Attempts to backtrack through forks and bitwidth modification operations diff --git a/lib/Transforms/BufferPlacement/BufferPlacementMILP.cpp b/lib/Transforms/BufferPlacement/BufferPlacementMILP.cpp index 7988cc0349..eaa6fc7c99 100644 --- a/lib/Transforms/BufferPlacement/BufferPlacementMILP.cpp +++ b/lib/Transforms/BufferPlacement/BufferPlacementMILP.cpp @@ -1127,6 +1127,24 @@ void BufferPlacementMILP::logResults(BufferPlacement &placement) { os.unindent(); os << "\n"; } + + os << "\n# =================== #\n"; + os << "# Unit Retimings #\n"; + os << "# =================== #\n\n"; + + // Log retimings of all units in all CFDFCs + for (auto [idx, cfdfcWithVars] : llvm::enumerate(vars.cfdfcVars)) { + auto [cf, cfVars] = cfdfcWithVars; + os << "Unit retimings of CFDFC #" << idx << ":\n"; + os.indent(); + for (auto &[op, unitVars] : cfVars.unitVars) { + os << getUniqueName(op) + << ": (in: " << unitVars.retIn.get(GRB_DoubleAttr_X) + << ", out: " << unitVars.retOut.get(GRB_DoubleAttr_X) << ")\n"; + } + os.unindent(); + os << "\n"; + } } void BufferPlacementMILP::initialize() { diff --git a/tools/hls-verifier/hls-verifier.cpp b/tools/hls-verifier/hls-verifier.cpp index d22d6f05ec..a5600e1de7 100644 --- a/tools/hls-verifier/hls-verifier.cpp +++ b/tools/hls-verifier/hls-verifier.cpp @@ -72,7 +72,7 @@ void generateModelsimScripts(const VerificationContext &ctx) { os << "eval vsim tb\n"; } os << "log -r *\n"; - os << "run -all\n"; + os << "run 10000ns -all \n"; os << "exit\n"; } diff --git a/tools/integration/run_spec_integration.py b/tools/integration/run_spec_integration.py index bf0d882201..126bdeb486 100644 --- a/tools/integration/run_spec_integration.py +++ b/tools/integration/run_spec_integration.py @@ -12,7 +12,9 @@ DYNAMATIC_ROOT = Path(__file__).parent.parent.parent INTEGRATION_FOLDER = DYNAMATIC_ROOT / "integration-test" +CLANGXX_BIN = DYNAMATIC_ROOT / "bin" / "clang++" DYNAMATIC_OPT_BIN = DYNAMATIC_ROOT / "build" / "bin" / "dynamatic-opt" +DYNAMATIC_PROFILER_BIN = DYNAMATIC_ROOT / "bin" / "exp-frequency-profiler" EXPORT_DOT_BIN = DYNAMATIC_ROOT / "build" / "bin" / "export-dot" EXPORT_RTL_BIN = DYNAMATIC_ROOT / "build" / "bin" / "export-rtl" SIMULATE_SH = DYNAMATIC_ROOT / "tools" / \ @@ -47,7 +49,7 @@ def color_print(string: str, color: str): print(f"{color}{string}{TermColors.ENDC}") -def run_test(c_file: str, spec: bool) -> bool: +def run_test(c_file: str, spec: bool, cp) -> bool: """ Runs the specified integration test. """ @@ -147,30 +149,12 @@ def run_test(c_file: str, spec: bool) -> bool: TermColors.FAIL) return False - # Buffer placement (Simple buffer placement) - handshake_buffered = os.path.join(comp_out_dir, "handshake_buffered.mlir") - timing_model = DYNAMATIC_ROOT / "data" / "components.json" - with open(handshake_buffered, "w") as f: - result = subprocess.run([ - DYNAMATIC_OPT_BIN, handshake_transformed, - "--handshake-set-buffering-properties=version=fpga20", - f"--handshake-place-buffers=algorithm=on-merges timing-models={timing_model}" - ], - stdout=f, - stderr=sys.stdout - ) - if result.returncode == 0: - print("Placed simple buffers") - else: - color_print("Failed to place simple buffers", TermColors.FAIL) - return False - # handshake canonicalization handshake_canonicalized = os.path.join( comp_out_dir, "handshake_canonicalized.mlir") with open(handshake_canonicalized, "w") as f: result = subprocess.run([ - DYNAMATIC_OPT_BIN, handshake_buffered, + DYNAMATIC_OPT_BIN, handshake_transformed, "--handshake-canonicalize", "--handshake-hoist-ext-instances" ], @@ -183,9 +167,8 @@ def run_test(c_file: str, spec: bool) -> bool: color_print("Failed to canonicalize Handshake", TermColors.FAIL) return False - handshake_export = os.path.join(comp_out_dir, "handshake_export.mlir") + # Speculation if spec: - # Speculation handshake_speculation = os.path.join( comp_out_dir, "handshake_speculation.mlir") spec_json = os.path.join(c_file_dir, "spec.json") @@ -204,34 +187,90 @@ def run_test(c_file: str, spec: bool) -> bool: else: color_print("Failed to add speculative units", TermColors.FAIL) return False + else: + handshake_speculation = handshake_canonicalized + + # Buffer placement (fpga20) + profiler_bin = os.path.join(comp_out_dir, "profile") + result = subprocess.run([ + CLANGXX_BIN, c_file, + "-D", "PRINT_PROFILING_INFO", + "-I", str(DYNAMATIC_ROOT / "include"), + "-Wno-deprecated", + "-o", profiler_bin + ]) + if result.returncode == 0: + print("Built kernel for profiling") + else: + print("Failed to place simple buffers") + + profiler_inputs = os.path.join(comp_out_dir, "profiler-inputs.txt") + with open(profiler_inputs, "w") as f: + result = subprocess.run([profiler_bin], + stdout=f, + stderr=sys.stdout + ) + if result.returncode == 0: + print("Ran kernel for profiling") + else: + print("Failed to kernel for profiling") + + frequencies = os.path.join(comp_out_dir, "frequencies.csv") + with open(frequencies, "w") as f: + result = subprocess.run([ + DYNAMATIC_PROFILER_BIN, cf_dyn_transformed, + "--top-level-function=" + kernel_name, + "--input-args-file=" + profiler_inputs, + ], + stdout=f, + stderr=sys.stdout + ) + if result.returncode == 0: + print("Profiled cf-level") + else: + print("Failed to profile cf-level") - buffer_json = os.path.join(c_file_dir, "buffer.json") - handshake_export = os.path.join(comp_out_dir, "handshake_export.mlir") - with open(buffer_json, "r") as f: - buffers = json.load(f) - buffer_pass_args = [] - for buffer in buffers: - buffer_pass_args.append( - "--handshake-placebuffers-custom=" + - f"pred={buffer['pred']} " + - f"outid={buffer['outid']} " + - f"slots={buffer['slots']} " + - f"type={buffer['type']}") - with open(handshake_export, "w") as f: - result = subprocess.run([ - DYNAMATIC_OPT_BIN, handshake_speculation, - *buffer_pass_args - ], - stdout=f, - stderr=sys.stdout - ) - if result.returncode == 0: - print("Exported Handshake") - else: - color_print("Failed to export Handshake", TermColors.FAIL) - return False + # Buffer placement (FPGA20) + handshake_buffered = os.path.join(comp_out_dir, "handshake_buffered.mlir") + timing_model = DYNAMATIC_ROOT / "data" / "components.json" + with open(handshake_buffered, "w") as f: + result = subprocess.run([ + DYNAMATIC_OPT_BIN, handshake_speculation, + "--handshake-set-buffering-properties=version=fpga20", + f"--handshake-place-buffers=algorithm=fpga20 frequencies={frequencies} timing-models={timing_model} target-period={cp} timeout=300 dump-logs" + ], + stdout=f, + stderr=sys.stdout, + cwd=comp_out_dir + ) + if result.returncode == 0: + print("Placed simple buffers") + else: + print("Failed to place simple buffers") + + # Speculation post-buffering + if spec: + handshake_spec_post_buffer = os.path.join( + comp_out_dir, "handshake_spec_post_buffer.mlir") + with open(handshake_spec_post_buffer, "w") as f: + result = subprocess.run([ + DYNAMATIC_OPT_BIN, handshake_buffered, + f"--handshake-spec-post-buffer", + "--handshake-materialize" + ], + stdout=f, + stderr=sys.stdout + ) + if result.returncode == 0: + print("Performed post-buffering speculation procedure") + else: + print("Failed to perform post-buffering speculation procedure") else: - shutil.copy(handshake_canonicalized, handshake_export) + handshake_spec_post_buffer = handshake_buffered + + handshake_export = os.path.join( + comp_out_dir, "handshake_export.mlir") + shutil.copy(handshake_spec_post_buffer, handshake_export) # Export dot file dot = os.path.join(comp_out_dir, f"{kernel_name}.dot") @@ -334,13 +373,16 @@ def main(): parser.add_argument( "--disable-spec", action="store_false", dest="spec", help="Run without speculation (but with custom compilation flow)") + parser.add_argument( + "--cp", type=str, help="clock period", default="10.000") args = parser.parse_args() test_name = args.test_name spec = args.spec + cp = args.cp success = run_test(INTEGRATION_FOLDER / test_name / - f"{test_name}.c", spec) + f"{test_name}.c", spec, cp) if success: color_print("Test passed", TermColors.OKGREEN) From 5e41c39aa058c53fa67f229f4659d83a9591abea Mon Sep 17 00:00:00 2001 From: shundroid Date: Fri, 17 Oct 2025 11:17:37 +0900 Subject: [PATCH 3/8] report bug --- data/verilog/handshake/control_merge.v | 3 +++ data/vhdl/handshake/control_merge.vhd | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/data/verilog/handshake/control_merge.v b/data/verilog/handshake/control_merge.v index 1352c288ab..b0fa7df0fb 100644 --- a/data/verilog/handshake/control_merge.v +++ b/data/verilog/handshake/control_merge.v @@ -38,5 +38,8 @@ assign index = index_internal; + initial begin + $fatal("control_merge implementation with data signal has a bug. Use beta backend instead"); + end assign outs = ins[index_internal * DATA_TYPE +: DATA_TYPE]; endmodule diff --git a/data/vhdl/handshake/control_merge.vhd b/data/vhdl/handshake/control_merge.vhd index 0dc5153598..4755b46858 100644 --- a/data/vhdl/handshake/control_merge.vhd +++ b/data/vhdl/handshake/control_merge.vhd @@ -48,5 +48,9 @@ begin ); index <= index_internal; + + assert false + report "control_merge implementation with data signal has a bug. Use beta backend instead" + severity failure; outs <= ins(to_integer(unsigned(index_internal))); end architecture; From 01f586eb44f584bc5c63561bd9edc1b91a06edab Mon Sep 17 00:00:00 2001 From: shundroid Date: Fri, 17 Oct 2025 11:21:46 +0900 Subject: [PATCH 4/8] fixed control_merge in beta backend --- .../generators/handshake/control_merge.py | 135 ++++++------------ 1 file changed, 43 insertions(+), 92 deletions(-) diff --git a/experimental/tools/unit-generators/vhdl/generators/handshake/control_merge.py b/experimental/tools/unit-generators/vhdl/generators/handshake/control_merge.py index c5b337fbd0..665436019a 100644 --- a/experimental/tools/unit-generators/vhdl/generators/handshake/control_merge.py +++ b/experimental/tools/unit-generators/vhdl/generators/handshake/control_merge.py @@ -4,8 +4,10 @@ from generators.support.signal_manager.utils.generation import generate_concat_and_handshake, generate_slice_and_handshake from generators.support.signal_manager.utils.types import ExtraSignals from generators.handshake.buffers.one_slot_break_r import generate_one_slot_break_r -from generators.handshake.merge_notehb import generate_merge_notehb from generators.handshake.fork import generate_fork +# TODO: Update the normal merge to merge_notehb +from generators.handshake.merge_notehb import generate_merge_notehb +from generators.support.utils import data def generate_control_merge(name, params): @@ -20,35 +22,36 @@ def generate_control_merge(name, params): if extra_signals: return _generate_control_merge_signal_manager(name, size, index_bitwidth, data_bitwidth, extra_signals) - elif data_bitwidth == 0: - return _generate_control_merge_dataless(name, size, index_bitwidth) else: return _generate_control_merge(name, size, index_bitwidth, data_bitwidth) -def _generate_control_merge_dataless(name, size, index_bitwidth): +def _generate_control_merge(name, size, index_bitwidth, data_bitwidth): merge_name = f"{name}_merge" one_slot_break_r_name = f"{name}_one_slot_break_r" fork_name = f"{name}_fork" - dependencies = generate_merge_notehb(merge_name, {"size": size}) + \ - generate_one_slot_break_r(one_slot_break_r_name, {"bitwidth": index_bitwidth}) + \ + dependencies = generate_merge_notehb(merge_name, {"size": size, "bitwidth": data_bitwidth}) + \ + generate_one_slot_break_r(one_slot_break_r_name, {"bitwidth": index_bitwidth + data_bitwidth}) + \ generate_fork(fork_name, {"size": 2, "bitwidth": 0}) entity = f""" library ieee; use ieee.std_logic_1164.all; use ieee.numeric_std.all; +use work.types.all; --- Entity of control_merge_dataless +-- Entity of control_merge entity {name} is port ( clk : in std_logic; rst : in std_logic; -- input channels + {data(f"ins : in data_array({size} - 1 downto 0)({data_bitwidth} - 1 downto 0);", data_bitwidth)} ins_valid : in std_logic_vector({size} - 1 downto 0); ins_ready : out std_logic_vector({size} - 1 downto 0); -- data output channel + {data(f"outs : out std_logic_vector({data_bitwidth} - 1 downto 0);", data_bitwidth)} outs_valid : out std_logic; outs_ready : in std_logic; -- index output channel @@ -60,17 +63,20 @@ def _generate_control_merge_dataless(name, size, index_bitwidth): """ architecture = f""" --- Architecture of control_merge_dataless +-- Architecture of control_merge architecture arch of {name} is - signal index_one_slot_break_r : std_logic_vector ({index_bitwidth} - 1 downto 0); - signal dataAvailable, readyToFork, one_slot_break_rOut_valid, one_slot_break_rOut_ready : std_logic; + {data(f"signal merge_outs : std_logic_vector({data_bitwidth} - 1 downto 0);", data_bitwidth)} + signal merge_outs_valid : std_logic; + signal buf_ins_ready, buf_outs_valid : std_logic; + signal fork_ins_ready : std_logic; + signal index_internal : std_logic_vector({index_bitwidth} - 1 downto 0); begin process (ins_valid) begin - index_one_slot_break_r <= ({index_bitwidth} - 1 downto 0 => '0'); + index_internal <= ({index_bitwidth} - 1 downto 0 => '0'); for i in 0 to ({size} - 1) loop if (ins_valid(i) = '1') then - index_one_slot_break_r <= std_logic_vector(to_unsigned(i, {index_bitwidth})); + index_internal <= std_logic_vector(to_unsigned(i, {index_bitwidth})); exit; end if; end loop; @@ -78,96 +84,41 @@ def _generate_control_merge_dataless(name, size, index_bitwidth): merge_ins : entity work.{merge_name}(arch) port map( - clk => clk, - rst => rst, - ins_valid => ins_valid, - outs_ready => one_slot_break_rOut_ready, - ins_ready => ins_ready, - outs_valid => dataAvailable + clk => clk, + rst => rst, + {data("ins => ins,", data_bitwidth)} + ins_valid => ins_valid, + ins_ready => ins_ready, + {data("outs => merge_outs,", data_bitwidth)} + outs_valid => merge_outs_valid, + outs_ready => buf_ins_ready ); one_slot_break_r : entity work.{one_slot_break_r_name}(arch) port map( - clk => clk, - rst => rst, - ins_valid => dataAvailable, - outs_ready => readyToFork, - outs_valid => one_slot_break_rOut_valid, - ins_ready => one_slot_break_rOut_ready, - ins => index_one_slot_break_r, - outs => index + clk => clk, + rst => rst, + ins({index_bitwidth - 1} downto 0) => index_internal, + {data(f"ins({data_bitwidth + index_bitwidth - 1} downto {index_bitwidth}) => merge_outs,", data_bitwidth)} + ins_valid => merge_outs_valid, + ins_ready => buf_ins_ready, + outs({index_bitwidth - 1} downto 0) => index, + {data(f"outs({data_bitwidth + index_bitwidth - 1} downto {index_bitwidth}) => outs,", data_bitwidth)} + outs_valid => buf_outs_valid, + outs_ready => fork_ins_ready ); fork_valid : entity work.{fork_name}(arch) port map( - clk => clk, - rst => rst, - ins_valid => one_slot_break_rOut_valid, - outs_ready(0) => outs_ready, - outs_ready(1) => index_ready, - ins_ready => readyToFork, + clk => clk, + rst => rst, + ins_valid => buf_outs_valid, + ins_ready => fork_ins_ready, outs_valid(0) => outs_valid, - outs_valid(1) => index_valid - ); -end architecture; -""" - - return dependencies + entity + architecture - - -def _generate_control_merge(name, size, index_bitwidth, data_bitwidth): - inner_name = f"{name}_inner" - - dependencies = _generate_control_merge_dataless( - inner_name, size, index_bitwidth) - - entity = f""" -library ieee; -use ieee.std_logic_1164.all; -use ieee.numeric_std.all; -use work.types.all; - --- Entity of control_merge -entity {name} is - port ( - clk : in std_logic; - rst : in std_logic; - -- input channels - ins : in data_array({size} - 1 downto 0)({data_bitwidth} - 1 downto 0); - ins_valid : in std_logic_vector({size} - 1 downto 0); - ins_ready : out std_logic_vector({size} - 1 downto 0); - -- data output channel - outs : out std_logic_vector({data_bitwidth} - 1 downto 0); - outs_valid : out std_logic; - outs_ready : in std_logic; - -- index output channel - index : out std_logic_vector({index_bitwidth} - 1 downto 0); - index_valid : out std_logic; - index_ready : in std_logic - ); -end entity; -""" - - architecture = f""" --- Architecture of control_merge -architecture arch of {name} is - signal index_internal : std_logic_vector({index_bitwidth} - 1 downto 0); -begin - control : entity work.{inner_name} - port map( - clk => clk, - rst => rst, - ins_valid => ins_valid, - ins_ready => ins_ready, - outs_valid => outs_valid, - outs_ready => outs_ready, - index => index_internal, - index_valid => index_valid, - index_ready => index_ready + outs_valid(1) => index_valid, + outs_ready(0) => outs_ready, + outs_ready(1) => index_ready ); - - index <= index_internal; - outs <= ins(to_integer(unsigned(index_internal))); end architecture; """ From bd784f7be04a1680223c6b97a9ed32ce1f088129 Mon Sep 17 00:00:00 2001 From: shundroid Date: Fri, 17 Oct 2025 18:11:33 +0900 Subject: [PATCH 5/8] refined implementation --- .../Speculation/HandshakeSpecPostBuffer.cpp | 165 +++++++++++------- .../Speculation/HandshakeSpeculation.cpp | 7 +- 2 files changed, 106 insertions(+), 66 deletions(-) diff --git a/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp b/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp index 948c4826c5..fb3743e721 100644 --- a/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp +++ b/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp @@ -9,6 +9,7 @@ #include "mlir/IR/OperationSupport.h" #include "mlir/IR/Value.h" #include "mlir/Support/LLVM.h" +#include "mlir/Support/LogicalResult.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/ErrorHandling.h" @@ -55,16 +56,11 @@ static handshake::ConditionalBranchOp findControlBranch(FuncOp funcOp, if (auto brBB = getLogicBB(condBrOp); !brBB || brBB != bb) continue; - llvm::errs() << "Candidate: "; - condBrOp->dump(); - for (Value result : condBrOp->getResults()) { for (Operation *user : result.getUsers()) { if (isBackedge(result, user)) return condBrOp; - llvm::errs() << "Rejecting user: "; - user->dump(); } } } @@ -72,36 +68,10 @@ static handshake::ConditionalBranchOp findControlBranch(FuncOp funcOp, return nullptr; } -void HandshakeSpecPostBufferPass::runDynamaticPass() { - ModuleOp modOp = getOperation(); - - // Support only one funcOp - assert(std::distance(modOp.getOps().begin(), - modOp.getOps().end()) == 1 && - "Expected a single FuncOp in the module"); - - FuncOp funcOp = *modOp.getOps().begin(); - - SpecPreBufferOp1 specOp1 = *funcOp.getOps().begin(); - SpecPreBufferOp2 specOp2 = *funcOp.getOps().begin(); - - unsigned specBB = getLogicBB(specOp1).value(); - - OpBuilder builder(&getContext()); - builder.setInsertionPoint(specOp1); - - SpeculatorOp speculator = builder.create( - specOp1.getLoc(), specOp1.getDataOut().getType(), specOp2.getDataIn(), - specOp1.getTrigger(), specOp1.getFifoDepth()); - setBB(speculator, specBB); - - // speculator.setConstant(constant); - // speculator.setDefaultValue(defaultValue); - - specOp1.getDataOut().replaceAllUsesWith(speculator.getDataOut()); - specOp2.getSaveCtrl().replaceAllUsesWith(speculator.getSaveCtrl()); - specOp2.getCommitCtrl().replaceAllUsesWith(speculator.getCommitCtrl()); - specOp2.getSCIsMisspec().replaceAllUsesWith(speculator.getSCIsMisspec()); +static FailureOr constructSaveCommitControl(SpeculatorOp speculator) { + OpBuilder builder(speculator.getContext()); + builder.setInsertionPoint(speculator); + unsigned specBB = getLogicBB(speculator).value(); // Construct Save Commit Control auto branchDiscardCondNonMisspec = cast( @@ -119,11 +89,12 @@ void HandshakeSpecPostBufferPass::runDynamaticPass() { SmallVector mergeOperands; mergeOperands.push_back(speculator.getSCSaveCtrl()); - ConditionalBranchOp controlBranch = findControlBranch(funcOp, specBB); + ConditionalBranchOp controlBranch = + findControlBranch(speculator->getParentOfType(), specBB); if (controlBranch == nullptr) { - specOp1->emitError() << "Could not find backedge within speculation bb: " - << specBB << ".\n"; - return signalPassFailure(); + speculator->emitError() + << "Could not find backedge within speculation bb: " << specBB << ".\n"; + return failure(); } // Helper function to check if a value leads to a Backedge @@ -150,48 +121,114 @@ void HandshakeSpecPostBufferPass::runDynamaticPass() { controlBranch->emitError() << "Could not find the backedge in the Control Branch " << specBB << "\n"; - return signalPassFailure(); + return failure(); } // All the inputs to the merge operation are ready auto mergeOp = builder.create(branchReplicated.getLoc(), mergeOperands); + mergeOp->setAttr("specv1_sc_merge", builder.getUnitAttr()); setBB(mergeOp, specBB); - specOp1.getSCSaveCtrl().replaceAllUsesWith(mergeOp.getResult()); + return mergeOp.getResult(); +} - specOp1->erase(); - specOp2->erase(); +static LogicalResult placeAdditionalBuffers(SpeculatorOp speculator) { + FuncOp funcOp = speculator->getParentOfType(); + OpBuilder builder(funcOp.getContext()); for (auto commitOp : funcOp.getOps()) { builder.setInsertionPoint(commitOp); - // To maintain high throughput: commit op *sometimes* joins a control with - // data from the iteration i-1. We need a 1-slot buffer to hold the control - // signal + // To maintain high throughput: commit op *sometimes* joins a control from + // the iteration `i` with data from the iteration `i-1`. We need a 1-slot + // buffer to hold the control signal auto bufOp1 = builder.create(builder.getUnknownLoc(), commitOp.getCtrl(), 1, BufferType::FIFO_BREAK_NONE); inheritBB(commitOp, bufOp1); bufOp1.getOperand().replaceAllUsesExcept(bufOp1.getResult(), bufOp1); - - // To avoid deadlock: On misspeculation, kill follows resend. When "resend" - // is sent, misspeculated data doesn't join with "kill" signal. The stall of - // this data possibly leads to a deadlock ("resend" is never accepted by a - // save-commit). Therefore, we need a 1-slot buffer to store the token - auto bufOp2 = - builder.create(builder.getUnknownLoc(), commitOp.getDataIn(), - 1, BufferType::FIFO_BREAK_NONE); - inheritBB(commitOp, bufOp2); - bufOp2.getOperand().replaceAllUsesExcept(bufOp2.getResult(), bufOp2); } - for (auto specBranchOp : funcOp.getOps()) { - Value specResult = specBranchOp.getTrueResult(); - builder.setInsertionPointAfterValue(specResult); - // Create a buffer to hold the delayed operand - auto bufOp = builder.create(builder.getUnknownLoc(), specResult, - 1, BufferType::FIFO_BREAK_NONE); - inheritBBFromValue(specResult, bufOp); - specResult.replaceAllUsesExcept(bufOp.getResult(), bufOp); + // To avoid deadlock: on misspeculation, `kill` is only sent after `resend` is + // accepted. Buffering algorithm ignores `resend`, and insufficient buffering + // may cause deadlock. We buffer dataOut and commitCtrl of speculator and the + // merged save-commit control for a `resend` iteration. + + auto bufDataOut = + builder.create(builder.getUnknownLoc(), speculator.getDataOut(), + 1, BufferType::FIFO_BREAK_NONE); + inheritBB(speculator, bufDataOut); + speculator.getDataOut().replaceAllUsesExcept(bufDataOut.getResult(), + bufDataOut); + + auto bufCommitCtrl = builder.create(builder.getUnknownLoc(), + speculator.getCommitCtrl(), 1, + BufferType::FIFO_BREAK_NONE); + inheritBB(speculator, bufCommitCtrl); + speculator.getCommitCtrl().replaceAllUsesExcept(bufCommitCtrl.getResult(), + bufCommitCtrl); + + MergeOp merge; + bool mergeFound = false; + for (auto candidate : funcOp.getOps()) { + if (candidate->hasAttr("specv1_sc_merge")) { + merge = candidate; + mergeFound = true; + break; + } + } + if (!mergeFound) { + funcOp.emitError("specv1_sc_merge not found"); + return failure(); } + auto bufSCControl = + builder.create(builder.getUnknownLoc(), merge.getResult(), 1, + BufferType::FIFO_BREAK_NONE); + inheritBB(speculator, bufSCControl); + merge.getResult().replaceAllUsesExcept(bufSCControl.getResult(), + bufSCControl); + + return success(); +} + +void HandshakeSpecPostBufferPass::runDynamaticPass() { + ModuleOp modOp = getOperation(); + + // Support only one funcOp + assert(std::distance(modOp.getOps().begin(), + modOp.getOps().end()) == 1 && + "Expected a single FuncOp in the module"); + + FuncOp funcOp = *modOp.getOps().begin(); + + SpecPreBufferOp1 specOp1 = *funcOp.getOps().begin(); + SpecPreBufferOp2 specOp2 = *funcOp.getOps().begin(); + + unsigned specBB = getLogicBB(specOp1).value(); + + OpBuilder builder(&getContext()); + builder.setInsertionPoint(specOp1); + + // Build the (post-buffer) SpeculatorOp + SpeculatorOp speculator = builder.create( + specOp1.getLoc(), specOp1.getDataOut().getType(), specOp2.getDataIn(), + specOp1.getTrigger(), specOp1.getFifoDepth()); + setBB(speculator, specBB); + + specOp1.getDataOut().replaceAllUsesWith(speculator.getDataOut()); + specOp2.getSaveCtrl().replaceAllUsesWith(speculator.getSaveCtrl()); + specOp2.getCommitCtrl().replaceAllUsesWith(speculator.getCommitCtrl()); + specOp2.getSCIsMisspec().replaceAllUsesWith(speculator.getSCIsMisspec()); + + auto scControl = constructSaveCommitControl(speculator); + if (failed(scControl)) + return signalPassFailure(); + + specOp1.getSCSaveCtrl().replaceAllUsesWith(scControl.value()); + + specOp1->erase(); + specOp2->erase(); + + if (failed(placeAdditionalBuffers(speculator))) + return signalPassFailure(); } diff --git a/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp b/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp index f54ffc3bbf..b965c95054 100644 --- a/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp +++ b/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp @@ -446,12 +446,13 @@ FailureOr HandshakeSpeculationPass::generateSaveCommitCtrl() { auto conditionOperand = controlBranch.getConditionOperand(); // trueResultType and falseResultType are tentative and will be updated in the // addSpecTag algorithm later. + // Operands are temporary. Will be updated at the end of the pass. auto branchDiscardCondNonSpec = builder.create( controlBranch.getLoc(), /*trueResultType=*/conditionOperand.getType(), /*falseResultType=*/conditionOperand.getType(), - /*specTag=*/conditionOperand, conditionOperand); + /*specTag=*/specOp1.getDataOut(), conditionOperand); inheritBB(specOp1, branchDiscardCondNonSpec); branchDiscardCondNonSpec->setAttr("specv1_branchDiscardCondNonSpec", builder.getUnitAttr()); @@ -846,7 +847,9 @@ void HandshakeSpeculationPass::runDynamaticPass() { if (failed(addNonSpecOp())) return signalPassFailure(); - // quick fix + // Quick fix: branchDiscardCondNonspec's operands must be the loop condition + // The real loop condition only turns out after the placement of speculative + // units (Speculator or save-commit unit may produce this) handshake::FuncOp funcOp = specOp1->getParentOfType(); for (auto branch : funcOp.getOps()) { if (branch->getAttr("specv1_branchDiscardCondNonSpec")) { From 9503cb785903d0bb93885d2ec3d5601eb7b68178 Mon Sep 17 00:00:00 2001 From: shundroid Date: Sat, 18 Oct 2025 16:01:27 +0900 Subject: [PATCH 6/8] updated integration test results --- integration-test/fixed/results.md | 4 ++-- integration-test/fixed/spec.json | 4 ++-- integration-test/if_convert/results.md | 4 ++-- integration-test/if_convert/spec.json | 4 ++-- integration-test/loop_path/results.md | 4 ++-- integration-test/nested_loop/results.md | 4 ++-- integration-test/single_loop/results.md | 2 +- integration-test/sparse/results.md | 4 ++-- integration-test/subdiag/results.md | 4 ++-- integration-test/subdiag/spec.json | 4 ++-- integration-test/subdiag_fast/results.md | 2 +- 11 files changed, 20 insertions(+), 20 deletions(-) diff --git a/integration-test/fixed/results.md b/integration-test/fixed/results.md index 86d9141780..a38eaa0c17 100644 --- a/integration-test/fixed/results.md +++ b/integration-test/fixed/results.md @@ -5,5 +5,5 @@ Since the variables `x0` and `x1` depend on values from previous iterations, the | | No Speculation | Speculation | |----------------------|------------------|-------------------| | II (Haoran’s thesis) | 16 | 6 | -| II | 14 | 5 | -| Cycles (Test Bench) | 705 (End: 704) | 270 (End: 265) | +| II | 13 | 4 | +| Cycles (Test Bench) | 657 | 216 | diff --git a/integration-test/fixed/spec.json b/integration-test/fixed/spec.json index 73252e6426..3e98fff1d9 100644 --- a/integration-test/fixed/spec.json +++ b/integration-test/fixed/spec.json @@ -2,7 +2,7 @@ "speculator": { "operation-name": "fork5", "operand-idx": 0, - "fifo-depth": 3 + "fifo-depth": 4 }, - "save-commits-fifo-depth": 3 + "save-commits-fifo-depth": 4 } diff --git a/integration-test/if_convert/results.md b/integration-test/if_convert/results.md index 717367c9ce..2de6d1e8f3 100644 --- a/integration-test/if_convert/results.md +++ b/integration-test/if_convert/results.md @@ -5,5 +5,5 @@ The optimal II was achieved with speculation. | | No Speculation | Speculation | |----------------------|------------------|-------------------| | II (Haoran’s thesis) | 7 | 1 | -| II | 6 | 1 | -| Cycles (Test Bench) | 1129 (End: 1127) | 309 (End: 307) | +| II | 5 | 1 | +| Cycles (Test Bench) | 943 | 315 | diff --git a/integration-test/if_convert/spec.json b/integration-test/if_convert/spec.json index ec0933582e..a808593393 100644 --- a/integration-test/if_convert/spec.json +++ b/integration-test/if_convert/spec.json @@ -2,7 +2,7 @@ "speculator": { "operation-name": "select0", "operand-idx": 0, - "fifo-depth": 6 + "fifo-depth": 7 }, - "save-commits-fifo-depth": 6 + "save-commits-fifo-depth": 7 } diff --git a/integration-test/loop_path/results.md b/integration-test/loop_path/results.md index 4462e6b3fe..aeca88e076 100644 --- a/integration-test/loop_path/results.md +++ b/integration-test/loop_path/results.md @@ -7,5 +7,5 @@ The II is only 2, even in the non-speculation case. This is because the break co | | No Speculation | Speculation | |----------------------|------------------|-------------------| | II (Haoran’s thesis) | 6 | 1 | -| II | 2 | 1 | -| Cycles (Test Bench) | 341 (End: 339) | 175 (End: 173) | +| II | 3 | 1 | +| Cycles (Test Bench) | 508 | 176 | diff --git a/integration-test/nested_loop/results.md b/integration-test/nested_loop/results.md index e4f477db12..c232c35fee 100644 --- a/integration-test/nested_loop/results.md +++ b/integration-test/nested_loop/results.md @@ -5,5 +5,5 @@ The result matched that of Haoran's thesis. | | No Speculation | Speculation | |----------------------------------------|------------------|-------------------| | II of the inner loop (Haoran’s thesis) | 6 | 1 | -| II of the inner loop | 6 | 1 | -| Cycles (Test Bench) | 2423 (End: 2421) | 437 (End: 433) | +| II of the inner loop | 5 | 1 | +| Cycles (Test Bench) | 2018 | 428 | diff --git a/integration-test/single_loop/results.md b/integration-test/single_loop/results.md index 7733df43e9..a141993a9d 100644 --- a/integration-test/single_loop/results.md +++ b/integration-test/single_loop/results.md @@ -6,4 +6,4 @@ The result matched that of Haoran's thesis. |----------------------|------------------|-------------------| | II (Haoran’s thesis) | 6 | 1 | | II | 6 | 1 | -| Cycles (Test Bench) | 3013 (End: 3011) | 516 (End: 511) | +| Cycles (Test Bench) | 3013 | 513 | diff --git a/integration-test/sparse/results.md b/integration-test/sparse/results.md index dd40173365..f2d8c0bdc9 100644 --- a/integration-test/sparse/results.md +++ b/integration-test/sparse/results.md @@ -5,5 +5,5 @@ Although Haoran's thesis attempted data speculation and achieved an II of 1, it | | No Speculation | Speculation | |----------------------|------------------|-------------------| | II (Haoran’s thesis) | 16 | 1 | -| II | 15 | 10 | -| Cycles (Test Bench) | 1237 (End: 1235) | 835 (End: 831) | +| II | 14 | 9 | +| Cycles (Test Bench) | 1155 | 750 | diff --git a/integration-test/subdiag/results.md b/integration-test/subdiag/results.md index ed82a09ce4..75328a90a2 100644 --- a/integration-test/subdiag/results.md +++ b/integration-test/subdiag/results.md @@ -7,5 +7,5 @@ Additionally, the current implementation of `cmpf` seems to generate output with | | No Speculation | Speculation | |----------------------|------------------|-------------------| | II (Haoran’s thesis) | 15 | 1 | -| II | 16 | 2 | -| Cycles (Test Bench) | 1623 (End: 1621) | 228 (End: 222) | +| II | 15 | 2 | +| Cycles (Test Bench) | 1522 | 223 | diff --git a/integration-test/subdiag/spec.json b/integration-test/subdiag/spec.json index e08516fe79..cd2bbd91e3 100644 --- a/integration-test/subdiag/spec.json +++ b/integration-test/subdiag/spec.json @@ -2,7 +2,7 @@ "speculator": { "operation-name": "fork3", "operand-idx": 0, - "fifo-depth": 8 + "fifo-depth": 9 }, - "save-commits-fifo-depth": 8 + "save-commits-fifo-depth": 9 } diff --git a/integration-test/subdiag_fast/results.md b/integration-test/subdiag_fast/results.md index 3a817d8486..9e36dcf0e2 100644 --- a/integration-test/subdiag_fast/results.md +++ b/integration-test/subdiag_fast/results.md @@ -5,4 +5,4 @@ In contrast to the original `subdiag`, it successfully achieved an II of 1. | | No Speculation | Speculation | |--------------------------|------------------|-------------------| | II | 15 | 1 | -| Cycles (Test Bench) | 1522 (End: 1520) | 126 (End: 120) | +| Cycles (Test Bench) | 1421 | 121 | From af910a1b45fd1a1a08415c55febe22ec6922ab07 Mon Sep 17 00:00:00 2001 From: shundroid Date: Sat, 18 Oct 2025 19:06:05 +0900 Subject: [PATCH 7/8] removed saveCtrl --- .../lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp | 1 - .../lib/Transforms/Speculation/HandshakeSpeculation.cpp | 3 --- include/dynamatic/Dialect/Handshake/HandshakeOps.td | 7 ++----- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp b/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp index fb3743e721..8adc06eaf6 100644 --- a/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp +++ b/experimental/lib/Transforms/Speculation/HandshakeSpecPostBuffer.cpp @@ -216,7 +216,6 @@ void HandshakeSpecPostBufferPass::runDynamaticPass() { setBB(speculator, specBB); specOp1.getDataOut().replaceAllUsesWith(speculator.getDataOut()); - specOp2.getSaveCtrl().replaceAllUsesWith(speculator.getSaveCtrl()); specOp2.getCommitCtrl().replaceAllUsesWith(speculator.getCommitCtrl()); specOp2.getSCIsMisspec().replaceAllUsesWith(speculator.getSCIsMisspec()); diff --git a/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp b/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp index b965c95054..15256692a4 100644 --- a/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp +++ b/experimental/lib/Transforms/Speculation/HandshakeSpeculation.cpp @@ -813,9 +813,6 @@ void HandshakeSpeculationPass::runDynamaticPass() { llvm::errs() << "Error: Placement of save units is not supported.\n"; return signalPassFailure(); } - // Place Save operations - // if (failed(placeUnits(this->specOp.getSaveCtrl()))) - // return signalPassFailure(); if (!placements.getPlacements().empty()) { // Generate Place SaveCommit operations and the SaveCommit control path diff --git a/include/dynamatic/Dialect/Handshake/HandshakeOps.td b/include/dynamatic/Dialect/Handshake/HandshakeOps.td index 24ac22bdc3..8affe5c2d6 100644 --- a/include/dynamatic/Dialect/Handshake/HandshakeOps.td +++ b/include/dynamatic/Dialect/Handshake/HandshakeOps.td @@ -1171,8 +1171,6 @@ def SpecPreBufferOp1 : Handshake_Op<"spec_prebuffer1", [ def SpecPreBufferOp2 : Handshake_Op<"spec_prebuffer2", [ HasValidSpecTag<"dataIn">, - IsSimpleHandshake<"saveCtrl">, - IsIntSizedChannel<1, "saveCtrl">, IsSimpleHandshake<"commitCtrl">, IsIntSizedChannel<1, "commitCtrl">, IsSimpleHandshake<"SCIsMisspec">, @@ -1184,12 +1182,11 @@ def SpecPreBufferOp2 : Handshake_Op<"spec_prebuffer2", [ let arguments = (ins ChannelType:$dataIn, UI32Attr:$fifoDepth); - let results = (outs ChannelType:$saveCtrl, ChannelType:$commitCtrl, + let results = (outs ChannelType:$commitCtrl, ChannelType:$SCIsMisspec); let assemblyFormat = [{ - $dataIn attr-dict `:` type($dataIn) `,` - type($saveCtrl) `,` type($commitCtrl) `,` + $dataIn attr-dict `:` type($dataIn) `,` type($commitCtrl) `,` type($SCIsMisspec) }]; From 03d83eed72dd9fae9f32a028b1db06d4cf5a35dc Mon Sep 17 00:00:00 2001 From: shundroid Date: Sat, 18 Oct 2025 19:14:20 +0900 Subject: [PATCH 8/8] fixed comments --- .../vhdl/generators/handshake/speculation/spec_save_commit.py | 2 +- .../vhdl/generators/handshake/speculation/speculator.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/spec_save_commit.py b/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/spec_save_commit.py index 5cb5f59e17..f26a9143f4 100644 --- a/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/spec_save_commit.py +++ b/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/spec_save_commit.py @@ -252,7 +252,7 @@ def _generate_spec_save_commit(name, bitwidth, fifo_depth): else -- if only filling but not emptying if (TailEn = '1') and (HeadEn = '0') then - -- if new tail index will reach head index + -- if new tail index will reach head index - 1 (ring buffer full) if ((Tail + 2) mod {fifo_depth} = Head) then Full <= '1'; end if; diff --git a/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/speculator.py b/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/speculator.py index 7aba46163a..0c4cb3b600 100644 --- a/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/speculator.py +++ b/experimental/tools/unit-generators/vhdl/generators/handshake/speculation/speculator.py @@ -713,8 +713,8 @@ def _generate_predFifo(name, bitwidth, fifo_depth): else -- if only filling but not emptying if (TailEn = '1') and (HeadEn = '0') then - -- if new tail index will reach head index - if ((Tail +2) mod {fifo_depth} = Head) then + -- if new tail index will reach head index - 1 (ring buffer full) + if ((Tail + 2) mod {fifo_depth} = Head) then Full <= '1'; end if; elsif (TailEn = '0') and (HeadEn = '1') then