[Pipeline] add 'go' signal to pipeline #5455
Conversation
mortbopet
force-pushed
the
dev/mpetersen/pipeline_go
branch
from
3c46836
to
34d0a28
Compare
|
CI failing due to lack of clock gate lowering which doesn't lower to an external module. |
mortbopet
force-pushed
the
dev/mpetersen/pipeline_go
branch
2 times, most recently
from
e735b54
to
5e4035b
Compare
|
Reverted back to a |
| auto reg = builder.create<seq::CompRegOp>( | ||
| stageTerminator->getLoc(), regIn.getType(), regIn, clock, regName, | ||
| reset, Value(), StringAttr()); | ||
| auto reg = builder.create<seq::CompRegClockEnabledOp>( |
There was a problem hiding this comment.
We probably don't want the CE on FPGAs. Unless it's being used to respond to backpressure (which I don't think it is), it is just a power optimization for ASICs. On FPGAs, it just adds fan-out and routing congestion.
There was a problem hiding this comment.
Good point. I'll change this to an input mux, and add the clock gating as a flag.
| // Build valid register. The valid register is always reset to 0. | ||
| auto validRegName = | ||
| builder.getStringAttr(stageRegPrefix.strref() + "_valid"); | ||
| rets.valid = builder.create<seq::CompRegOp>( |
There was a problem hiding this comment.
How does this interact with stall? It has to just stay in place, right?
This looks fine for non-stalled pipelines.
There was a problem hiding this comment.
| @@ -262,34 +323,23 @@ LogicalResult ReturnOp::verify() { | |||
| // StageOp | |||
| //===----------------------------------------------------------------------===// | |||
|
|
|||
| SuccessorOperands StageOp::getSuccessorOperands(unsigned index) { | |||
There was a problem hiding this comment.
Does this have anything to do with adding go? Doesn't look like it...
There was a problem hiding this comment.
Implicitly, it does. StageOp no longer can comply with the getSuccessorOperands since it doesn't provide an operand for the s#_valid block argument to a given block - hence the removal.
This commit adds an explicit 'go' signal to the pipeline abstraction. This signal is used to indicate when the pipeline should start. The signal will propagate through each stage as the stage valid signal. As a result of this, each stage now has a `s#_valid : i1` signal as the last value in its block argument list. This value can be used within each stage for any operations which requires access to the pipeline control circuitry. Given that pipeline stage validity is now explicit within the block arguments of a stage, a `pipeline.stage` operation no longer has an `enable` signal. As a small 'bugfix' here, a `seq.clock_gate` is emitted to gate the pipeline stage separating registers on the pipeline stage valid signal.
mortbopet
force-pushed
the
dev/mpetersen/pipeline_go
branch
from
eba4791
to
cfeb2e9
Compare
This commit adds an explicit 'go' signal to the pipeline abstraction. This signal is used to indicate when the pipeline should start. The signal will propagate through each stage as the stage valid signal. As a result of this, each stage now has a
s#_valid : i1signal as the last value in its block argument list. This value can be used within each stage for any operations which requires access to the pipeline control circuitry.Given that pipeline stage validity is now explicit within the block arguments of a stage, a
pipeline.stageoperation no longer has anenablesignal. As a small 'bugfix' here, aseq.clock_gateis emitted to gate the pipeline stage separating registers on the pipeline stage valid signal.