Skip to content

Commit

Permalink
[FIRRTL] Change the FIRRTL dialect to require def-before-use in modules.
Browse files Browse the repository at this point in the history
This matches the expectation of the SFC compiler, and LowerTypes/LowerToRTL
require this.

This patch xfails the HandshakeToFIRRTL/test_load.mlir test, which is tracked
in Issue #534.
  • Loading branch information
lattner committed Jan 30, 2021
1 parent 59866d4 commit bf56ef1
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 4 deletions.
11 changes: 11 additions & 0 deletions docs/RationaleFIRRTL.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,17 @@ bundle result turn into its own distinct result on the `firrtl.instance`
operation. This is made possible by MLIR's robust support for multiple value
operands, and makes the IR much easier to analyze and work with.

#### Module bodies require def-before-use dominance instead of allowing graphs

MLIR allows regions with arbitrary graphs in their bodies, and this is used by
the RTL dialect to allow direct expression of cyclic graphs etc. While this
makes sense for hardware in general, the FIRRTL dialect is intended to be a
pragmatic infrastructure focused on lowering of Chisel code to the RTL dialect,
it isn't intended to be a "generally useful IR for hardware".

We recommend that non-Chisel frontends target the RTL dialect, or a higher level
dialect of their own creation that lowers to RTL as appropriate.

#### `input` and `output` Module Ports

The FIRRTL specification describes two kinds of ports: `input` and `output`.
Expand Down
5 changes: 1 addition & 4 deletions include/circt/Dialect/FIRRTL/FIRRTLStructure.td
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def CircuitOp : FIRRTLOp<"circuit",
}

def FModuleOp : FIRRTLOp<"module",
[IsolatedFromAbove, FunctionLike, Symbol, RegionKindInterface,
[IsolatedFromAbove, FunctionLike, Symbol,
SingleBlockImplicitTerminator<"DoneOp">]> {
let summary = "FIRRTL Module";
let description = [{
Expand All @@ -68,9 +68,6 @@ def FModuleOp : FIRRTLOp<"module",
using FunctionLike::front;
using FunctionLike::getBody;

// Implement RegionKindInterface.
static RegionKind getRegionKind(unsigned index) { return RegionKind::Graph;}

// Decode information about the input and output ports on this module.
void getPortInfo(SmallVectorImpl<ModulePortInfo> &results) {
getModulePortInfo(*this, results);
Expand Down
1 change: 1 addition & 0 deletions test/Conversion/HandshakeToFIRRTL/test_load.mlir
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: circt-opt -lower-handshake-to-firrtl -split-input-file -verify-diagnostics %s
// XFAIL: *

// CHECK-LABEL: firrtl.module @handshake_load_3ins_2outs_ui8(
// CHECK-SAME: %arg0: !firrtl.bundle<valid: uint<1>, ready: flip<uint<1>>, data: uint<64>>, %arg1: !firrtl.bundle<valid: uint<1>, ready: flip<uint<1>>, data: uint<8>>, %arg2: !firrtl.bundle<valid: uint<1>, ready: flip<uint<1>>>, %arg3: !firrtl.bundle<valid: flip<uint<1>>, ready: uint<1>, data: flip<uint<8>>>, %arg4: !firrtl.bundle<valid: flip<uint<1>>, ready: uint<1>, data: flip<uint<64>>>) {
Expand Down
11 changes: 11 additions & 0 deletions test/Dialect/FIRRTL/errors.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -373,3 +373,14 @@ firrtl.circuit "StructCast3" {
%b = firrtl.rtlStructCast %a : (!firrtl.bundle<valid: uint<1>>) -> (!rtl.struct<valid: i2>)
}
}

// -----

firrtl.circuit "OutOfOrder" {
firrtl.module @OutOfOrder(%a: !firrtl.uint<32>) {
// expected-error @+1 {{operand #0 does not dominate this use}}
%0 = firrtl.add %1, %1 : (!firrtl.uint<33>, !firrtl.uint<33>) -> !firrtl.uint<34>
// expected-note @+1 {{operand defined here}}
%1 = firrtl.add %a, %a : (!firrtl.uint<32>, !firrtl.uint<32>) -> !firrtl.uint<33>
}
}

0 comments on commit bf56ef1

Please sign in to comment.