Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LLHD] Make process lowering best-effort and allow constants from outside the region #7617

Merged
merged 2 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[LLHD] Make process lowering best-effort and allow constants outside …
…the region
  • Loading branch information
maerhart committed Sep 23, 2024
commit 23039afbd331c9258d6e77b0e0c16dcf6c6f4650
107 changes: 73 additions & 34 deletions lib/Dialect/LLHD/Transforms/ProcessLoweringPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
#include "mlir/IR/PatternMatch.h"
#include "mlir/IR/Visitors.h"
#include "mlir/Pass/Pass.h"
#include "llvm/Support/Debug.h"

#define DEBUG_TYPE "llhd-process-lowering"

namespace circt {
namespace llhd {
Expand All @@ -38,31 +41,46 @@ static LogicalResult isProcValidToLower(llhd::ProcessOp op) {
size_t numBlocks = op.getBody().getBlocks().size();

if (numBlocks == 1) {
if (!isa<llhd::HaltOp>(op.getBody().back().getTerminator()))
return op.emitOpError("during process-lowering: entry block is required "
"to be terminated by llhd.halt");
if (!isa<llhd::HaltOp>(op.getBody().back().getTerminator())) {
LLVM_DEBUG({
llvm::dbgs()
<< "entry block is required to be terminated by llhd.halt\n";
});
return failure();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it legal to inline a process with a halt terminator into the parent module? Wouldn't the process only execute once, but the inlined version would be continuously executed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, let me remove this case. Not sure why it was there to begin with.


return success();
}

if (numBlocks == 2) {
Block &first = op.getBody().front();
Block &last = op.getBody().back();

if (!last.getArguments().empty())
return op.emitOpError(
"during process-lowering: the second block (containing the "
"llhd.wait) is not allowed to have arguments");
if (!last.getArguments().empty()) {
LLVM_DEBUG({
llvm::dbgs() << "the second block (containing the "
"llhd.wait) is not allowed to have arguments\n";
});
return failure();
}

if (!isa<cf::BranchOp>(first.getTerminator()))
return op.emitOpError("during process-lowering: the first block has to "
"be terminated by a cf.br operation");
if (!isa<cf::BranchOp>(first.getTerminator())) {
LLVM_DEBUG({
llvm::dbgs() << "the first block has to "
"be terminated by a cf.br operation\n";
});
return failure();
}

if (auto wait = dyn_cast<llhd::WaitOp>(last.getTerminator())) {
// No optional time argument is allowed
if (wait.getTime())
return wait.emitOpError(
"during process-lowering: llhd.wait terminators with optional time "
"argument cannot be lowered to structural LLHD");
if (wait.getTime()) {
LLVM_DEBUG({
llvm::dbgs() << "llhd.wait terminators with optional time "
"argument cannot be lowered to structural LLHD\n";
});
return failure();
}

SmallVector<Value> observedSignals;
for (Value obs : wait.getObserved())
Expand All @@ -75,37 +93,61 @@ static LogicalResult isProcValidToLower(llhd::ProcessOp op) {
WalkResult result = op.walk([&](Operation *operation) -> WalkResult {
// TODO: value does not need to be observed if all values this value is
// a combinatorial result of are observed.
for (Value operand : operation->getOperands())
if (!op.getBody().isAncestor(operand.getParentRegion()) &&
!llvm::is_contained(wait.getObserved(), operand) &&
!llvm::is_contained(observedSignals, operand))
return wait.emitOpError(
"during process-lowering: the wait terminator is required to "
"have values used in the process as arguments");
for (Value operand : operation->getOperands()) {
if (op.getBody().isAncestor(operand.getParentRegion()))
continue;
if (llvm::is_contained(wait.getObserved(), operand))
continue;
if (llvm::is_contained(observedSignals, operand))
continue;
if (auto *defOp = operand.getDefiningOp();
defOp && defOp->hasTrait<OpTrait::ConstantLike>())
continue;
if (auto bitcastOp = operand.getDefiningOp<hw::BitcastOp>())
if (auto *defOp = bitcastOp.getInput().getDefiningOp();
defOp && defOp->hasTrait<OpTrait::ConstantLike>())
continue;

LLVM_DEBUG({
llvm::dbgs() << "the wait terminator is required to "
"have values used in the process as arguments\n";
});
return failure();
}

return WalkResult::advance();
});

return failure(result.wasInterrupted());
}

return op.emitOpError("during process-lowering: the second block must be "
"terminated by llhd.wait");
LLVM_DEBUG({
llvm::dbgs() << "the second block must be "
"terminated by llhd.wait\n";
});
return failure();
}

return op.emitOpError(
"process-lowering only supports processes with either one basic block "
"terminated by a llhd.halt operation or two basic blocks where the first "
"one contains a cf.br terminator and the second one is terminated by a "
"llhd.wait operation");
LLVM_DEBUG({
llvm::dbgs() << "process-lowering only supports processes with either "
"one basic block "
"terminated by a llhd.halt operation or two basic blocks "
"where the first "
"one contains a cf.br terminator and the second one is "
"terminated by a "
"llhd.wait operation\n";
});
return failure();
}

void ProcessLoweringPass::runOnOperation() {
ModuleOp module = getOperation();

WalkResult result = module.walk([](llhd::ProcessOp op) -> WalkResult {
module.walk([](llhd::ProcessOp op) {
LLVM_DEBUG({ llvm::dbgs() << "\n=== Process\n"; });
// Check invariants
if (failed(isProcValidToLower(op)))
return WalkResult::interrupt();
return;

// In the case that wait is used to suspend the process, we need to merge
// the two blocks as we needed the second block to have a target for wait
Expand All @@ -130,11 +172,8 @@ void ProcessLoweringPass::runOnOperation() {
rewriter.inlineBlockBefore(&op.getBody().front(), op);
op.erase();

return WalkResult::advance();
LLVM_DEBUG({ llvm::dbgs() << "Process lowered successfully!\n"; });
});

if (result.wasInterrupted())
signalPassFailure();
}
} // namespace

Expand Down
88 changes: 87 additions & 1 deletion test/Dialect/LLHD/Transforms/processLowering.mlir
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: circt-opt %s -llhd-process-lowering -split-input-file -verify-diagnostics | FileCheck %s
// RUN: circt-opt %s -llhd-process-lowering | FileCheck %s

// check that input and output signals are transferred correctly
// CHECK-LABEL: hw.module @inputAndOutput
Expand Down Expand Up @@ -107,3 +107,89 @@ hw.module @partialSignal(inout %arg0 : i64) {
llhd.wait (%1 : i64), ^bb1
}
}

// COM: Tests to check that non-combinational processes are not inlined.

// Check wait with observing probed signals
// CHECK-LABEL: @prbAndWaitNotObserved
// CHECK: llhd.process
hw.module @prbAndWaitNotObserved(inout %arg0 : i64) {
llhd.process {
cf.br ^bb1
^bb1:
%0 = llhd.prb %arg0 : !hw.inout<i64>
llhd.wait ^bb1
}
}

// Check that block arguments for the second block are not allowed.
// CHECK-LABEL: @blockArgumentsNotAllowed
// CHECK: llhd.process
hw.module @blockArgumentsNotAllowed(inout %arg0 : i64) {
llhd.process {
%prb = llhd.prb %arg0 : !hw.inout<i64>
cf.br ^bb1(%prb : i64)
^bb1(%a : i64):
llhd.wait ^bb1(%a: i64)
}
}

// Check that the entry block is terminated by a cf.br terminator.
// CHECK-LABEL: @entryBlockMustHaveBrTerminator
// CHECK: llhd.process
hw.module @entryBlockMustHaveBrTerminator() {
llhd.process {
llhd.wait ^bb1
^bb1:
llhd.wait ^bb1
}
}

// Check that there is no optional time operand in the wait terminator.
// CHECK-LABEL: @noOptionalTime
// CHECK: llhd.process
hw.module @noOptionalTime() {
llhd.process {
cf.br ^bb1
^bb1:
%time = llhd.constant_time #llhd.time<0ns, 0d, 0e>
llhd.wait for %time, ^bb1
}
}

// Check that if there are two blocks, the second one is terminated by a wait terminator.
// CHECK-LABEL: @secondBlockTerminatedByWait
// CHECK: llhd.process
hw.module @secondBlockTerminatedByWait() {
llhd.process {
cf.br ^bb1
^bb1:
llhd.halt
}
}

// Check that there are not more than two blocks.
// CHECK-LABEL: @moreThanTwoBlocksNotAllowed
// CHECK: llhd.process
hw.module @moreThanTwoBlocksNotAllowed() {
llhd.process {
cf.br ^bb1
^bb1:
cf.br ^bb2
^bb2:
llhd.wait ^bb1
}
}

// CHECK-LABEL: @muxedSignalNotLowerable
// CHECK: llhd.process
hw.module @muxedSignalNotLowerable(inout %arg0 : i64, inout %arg1 : i64, inout %arg2 : i1) {
llhd.process {
cf.br ^bb1
^bb1:
%cond = llhd.prb %arg2 : !hw.inout<i1>
%sig = comb.mux %cond, %arg0, %arg1 : !hw.inout<i64>
%0 = llhd.prb %sig : !hw.inout<i64>
llhd.wait (%cond : i1), ^bb1
}
}
90 changes: 0 additions & 90 deletions test/Dialect/LLHD/Transforms/processLoweringErrors.mlir

This file was deleted.

Loading