Skip to content

Commit

Permalink
Teach the mlir asmprinter to use the name attribute (when present) to
Browse files Browse the repository at this point in the history 
name the result SSA values, this makes the asmdumps waaay more readable.
  • Loading branch information
@lattner
lattner committed Apr 8, 2020
1 parent 2c987c3 commit 0e95d97
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 40 deletions.
7 changes: 5 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,16 @@ reasonably fast and is good to provide a sanity check that things are working):
$ cd ~/Projects/llvm-project
$ mkdir build
$ cd build
$ cmake -G Ninja ../llvm -DLLVM_EXTERNAL_PROJECTS="spt" -DLLVM_EXTERNAL_SPT_SOURCE_DIR=/Users/chrisl/Projects/spt -DLLVM_ENABLE_PROJECTS="mlir;spt" -DLLVM_TARGETS_TO_BUILD="X86;RISCV" -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DCMAKE_BUILD_TYPE=DEBUG
$ cmake -G Ninja ../llvm -DLLVM_EXTERNAL_PROJECTS="spt" -DLLVM_EXTERNAL_SPT_SOURCE_DIR=/Users/chrisl/Projects/spt -DLLVM_ENABLE_PROJECTS="mlir;spt" -DLLVM_TARGETS_TO_BUILD="X86;RISCV" -DLLVM_ENABLE_ASSERTIONS=ON -DCMAKE_BUILD_TYPE=DEBUG
```

The "-DCMAKE_BUILD_TYPE=DEBUG" flag enables debug information, which makes the
The `-DCMAKE_BUILD_TYPE=DEBUG` flag enables debug information, which makes the
whole tree compile slower, but allows you to step through code into the LLVM
and MLIR frameworks.

To get something that runs fast, use `-DCMAKE_BUILD_TYPE=Release`. This can
make a very large difference in performance.

5) Build MLIR and run MLIR tests as a smoketest:

```
Expand Down
15 changes: 9 additions & 6 deletions lib/Dialect/FIRRTL/Dialect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ using namespace firrtl;
//===----------------------------------------------------------------------===//

// If the specified attribute set contains the firrtl.name attribute, return it.
static StringAttr getFIRRTLNameAttr(ArrayRef<NamedAttribute> attrs) {
static StringAttr getModuleFIRRTLNameAttr(ArrayRef<NamedAttribute> attrs) {
for (auto &argAttr : attrs) {
// FIXME: We currently use firrtl.name instead of name because this makes
// the FunctionLike handling in MLIR core happier. It otherwise doesn't
// allow attributes on module parameters.
if (!argAttr.first.is("firrtl.name"))
continue;

Expand All @@ -30,7 +33,7 @@ static StringAttr getFIRRTLNameAttr(ArrayRef<NamedAttribute> attrs) {
namespace {

// We implement the OpAsmDialectInterface so that FIRRTL dialect operations
// automatically interpret the firrtl.name attribute on function arguments and
// automatically interpret the name attribute on function arguments and
// on operations as their SSA name.
struct FIRRTLOpAsmDialectInterface : public OpAsmDialectInterface {
using OpAsmDialectInterface::OpAsmDialectInterface;
Expand All @@ -40,7 +43,7 @@ struct FIRRTLOpAsmDialectInterface : public OpAsmDialectInterface {
void getAsmResultNames(Operation *op,
OpAsmSetValueNameFn setNameFn) const override {
if (op->getNumResults() > 0)
if (auto nameAttr = op->getAttrOfType<StringAttr>("firrtl.name"))
if (auto nameAttr = op->getAttrOfType<StringAttr>("name"))
setNameFn(op->getResult(0), nameAttr.getValue());
}

Expand All @@ -54,7 +57,7 @@ struct FIRRTLOpAsmDialectInterface : public OpAsmDialectInterface {

for (size_t i = 0, e = block->getNumArguments(); i != e; ++i) {
// Scan for a 'firrtl.name' attribute.
if (auto str = getFIRRTLNameAttr(impl::getArgAttrs(parentOp, i)))
if (auto str = getModuleFIRRTLNameAttr(impl::getArgAttrs(parentOp, i)))
setNameFn(block->getArgument(i), str.getValue());
}
}
Expand Down Expand Up @@ -150,7 +153,7 @@ void firrtl::getModulePortInfo(Operation *op,
for (unsigned i = 0, e = argTypes.size(); i < e; ++i) {
auto argAttrs = ::mlir::impl::getArgAttrs(op, i);
results.push_back(
{getFIRRTLNameAttr(argAttrs), argTypes[i].cast<FIRRTLType>()});
{getModuleFIRRTLNameAttr(argAttrs), argTypes[i].cast<FIRRTLType>()});
}
}

Expand Down Expand Up @@ -238,7 +241,7 @@ static void printFunctionSignature2(OpAsmPrinter &p, Operation *op,
ArrayRef<StringRef> elidedAttrs;
StringRef tmp;
if (argumentValue) {
if (auto nameAttr = getFIRRTLNameAttr(argAttrs)) {
if (auto nameAttr = getModuleFIRRTLNameAttr(argAttrs)) {

// Check to make sure the asmprinter is printing it correctly.
SmallString<32> resultNameStr;
Expand Down
52 changes: 26 additions & 26 deletions test/fir/basic.fir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,19 @@ circuit basic : ; CHECK: firrtl.circuit "basic" {
output auto : UInt<1> ; CHECK: %auto: !firrtl.flip<uint<1>>,
input i8 : UInt<8> ; CHECK: %i8: !firrtl.uint<8>)

; CHECK: %0 = firrtl.wire {name = "_T"} : !firrtl.vector<uint<1>, 12>
; CHECK: %_T = firrtl.wire {name = "_T"} : !firrtl.vector<uint<1>, 12>
wire _T : UInt<1>[12] @[Nodes.scala 370:76]

; CHECK: %1 = firrtl.wire {name = "_T_2"} : !firrtl.vector<uint<1>, 13>
; CHECK: %_T_2 = firrtl.wire {name = "_T_2"} : !firrtl.vector<uint<1>, 13>
wire _T_2 : UInt<1>[13]

; CHECK: %2 = firrtl.wire {name = "out_0"} : !firrtl.bundle<member: bundle<0: bundle<clock: clock, reset: uint<1>>>>
; CHECK: %out_0 = firrtl.wire {name = "out_0"} : !firrtl.bundle<member: bundle<0: bundle<clock: clock, reset: uint<1>>>>
wire out_0 : { member : { 0 : { clock : Clock, reset : UInt<1>}}}

; CHECK: firrtl.connect %0, %1 : !firrtl.vector<uint<1>, 12>, !firrtl.vector<uint<1>, 13>
; CHECK: firrtl.connect %_T, %_T_2 : !firrtl.vector<uint<1>, 12>, !firrtl.vector<uint<1>, 13>
_T <= _T_2

; CHECK: firrtl.partialconnect %0, %1
; CHECK: firrtl.partialconnect %_T, %_T_2
_T <- _T_2

; CHECK: firrtl.invalid %auto : !firrtl.flip<uint<1>>
Expand All @@ -73,18 +73,18 @@ circuit basic : ; CHECK: firrtl.circuit "basic" {
; CHECK: firrtl.invalid %reset : !firrtl.uint<1>
reset is invalid

; CHECK: %3 = firrtl.subfield %2("member") : (!firrtl.bundle<member: bundle<0: bundle<clock: clock, reset: uint<1>>>>) -> !firrtl.bundle<0: bundle<clock: clock, reset: uint<1>>>
; CHECK: %4 = firrtl.subfield %3("0") : (!firrtl.bundle<0: bundle<clock: clock, reset: uint<1>>>) -> !firrtl.bundle<clock: clock, reset: uint<1>>
; CHECK: %5 = firrtl.subfield %4("reset") : (!firrtl.bundle<clock: clock, reset: uint<1>>) -> !firrtl.uint<1>
; CHECK: firrtl.partialconnect %auto, %5 : !firrtl.flip<uint<1>>, !firrtl.uint<1>
; CHECK: %0 = firrtl.subfield %out_0("member") : (!firrtl.bundle<member: bundle<0: bundle<clock: clock, reset: uint<1>>>>) -> !firrtl.bundle<0: bundle<clock: clock, reset: uint<1>>>
; CHECK: %1 = firrtl.subfield %0("0") : (!firrtl.bundle<0: bundle<clock: clock, reset: uint<1>>>) -> !firrtl.bundle<clock: clock, reset: uint<1>>
; CHECK: %2 = firrtl.subfield %1("reset") : (!firrtl.bundle<clock: clock, reset: uint<1>>) -> !firrtl.uint<1>
; CHECK: firrtl.partialconnect %auto, %2 : !firrtl.flip<uint<1>>, !firrtl.uint<1>
auto <- out_0.member.0.reset @[Field 173:49]

; CHECK: %6 = firrtl.subindex %1[0] : (!firrtl.vector<uint<1>, 13>) -> !firrtl.uint<1>
; CHECK: %7 = firrtl.subindex %0[0] : (!firrtl.vector<uint<1>, 12>) -> !firrtl.uint<1>
; CHECK: firrtl.connect %6, %7
; CHECK: %3 = firrtl.subindex %_T_2[0] : (!firrtl.vector<uint<1>, 13>) -> !firrtl.uint<1>
; CHECK: %4 = firrtl.subindex %_T[0] : (!firrtl.vector<uint<1>, 12>) -> !firrtl.uint<1>
; CHECK: firrtl.connect %3, %4
_T_2[0] <= _T[0] @[Xbar.scala 21:44]

; CHECK: %8 = firrtl.node %auto {name = "n1"} : !firrtl.flip<uint<1>>
; CHECK: %n1 = firrtl.node %auto {name = "n1"} : !firrtl.flip<uint<1>>
node n1 = auto

; CHECK: firrtl.add %reset, %reset : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<2>
Expand All @@ -100,18 +100,18 @@ circuit basic : ; CHECK: firrtl.circuit "basic" {
auto <= add(UInt<10>(42), UInt<8>("hAB"))

; CHECK: firrtl.when %reset {
; CHECK: firrtl.connect %0, %1
; CHECK: firrtl.connect %_T, %_T_2
; CHECK: } else {
; CHECK: firrtl.partialconnect %0
; CHECK: firrtl.partialconnect %_T, %_T_2
; CHECK: }
when reset : _T <= _T_2 else : _T <- _T_2

; CHECK: firrtl.when %reset {
; CHECK: [[N4A:%.+]] = firrtl.node %1
; CHECK: firrtl.connect %0, [[N4A]]
; CHECK: [[N4A:%.+]] = firrtl.node %_T_2
; CHECK: firrtl.connect %_T, [[N4A]]
; CHECK: } else {
; CHECK: [[N4B:%.+]] = firrtl.node %1
; CHECK: firrtl.partialconnect %0, [[N4B]]
; CHECK: [[N4B:%.+]] = firrtl.node %_T_2
; CHECK: firrtl.partialconnect %_T, [[N4B]]
; CHECK: }
when reset :
node n4 = _T_2
Expand All @@ -123,13 +123,13 @@ circuit basic : ; CHECK: firrtl.circuit "basic" {
; CHECK: [[TMP:%.+]] = firrtl.constant 4
; CHECK: [[COND:%.+]] = firrtl.lt %reset, [[TMP]]
; CHECK: firrtl.when [[COND]] {
; CHECK: firrtl.connect %0, %1
; CHECK: firrtl.connect %_T, %_T_2
; CHECK: }
; CHECK-NOT: else
when lt(reset, UInt(4)) : ;; When with no else.
_T <= _T_2

; CHECK: firrtl.printf %clock, %reset, "\22Something interesting! %x %x\22" %0, %1 : !firrtl.vector<uint<1>, 12>, !firrtl.vector<uint<1>, 13>
; CHECK: firrtl.printf %clock, %reset, "\22Something interesting! %x %x\22" %_T, %_T_2 : !firrtl.vector<uint<1>, 12>, !firrtl.vector<uint<1>, 13>
printf(clock, reset, "Something interesting! %x %x", _T, _T_2)

; CHECK: firrtl.stop %clock, %reset, 42
Expand All @@ -142,20 +142,20 @@ circuit basic : ; CHECK: firrtl.circuit "basic" {
; CHECK: firrtl.shr %i8, 8 : (!firrtl.uint<8>) -> !firrtl.uint<1>
node n5 = or(shl(i8, 4), shr(i8, 8))

; CHECK: firrtl.dshl %i8, %24 : (!firrtl.uint<8>, !firrtl.uint<4>) -> !firrtl.uint<23>
; CHECK: firrtl.dshl %i8, %{{.*}} : (!firrtl.uint<8>, !firrtl.uint<4>) -> !firrtl.uint<23>
node n6 = dshl(i8, UInt<4>(7))

; CHECK: firrtl.cat %23, %26 : (!firrtl.uint<12>, !firrtl.uint<23>) -> !firrtl.uint<35>
; CHECK: firrtl.cat %{{.*}}, %{{.*}} : (!firrtl.uint<12>, !firrtl.uint<23>) -> !firrtl.uint<35>
node n7 = cat(n5, n6)

; CHECK: firrtl.mux(%reset, %i8, %29) : (!firrtl.uint<1>, !firrtl.uint<8>, !firrtl.uint) -> !firrtl.uint
; CHECK: firrtl.mux(%reset, %i8, %{{.*}}) : (!firrtl.uint<1>, !firrtl.uint<8>, !firrtl.uint) -> !firrtl.uint
node n8 = mux(reset, i8, UInt(4))

; CHECK: firrtl.reginit %clock, %reset, %32 {name = "_T_2621"} : (!firrtl.clock, !firrtl.uint<1>, !firrtl.uint<4>) -> !firrtl.uint<4>
; CHECK: firrtl.reginit %clock, %reset, %{{.*}} {name = "_T_2621"} : (!firrtl.clock, !firrtl.uint<1>, !firrtl.uint<4>) -> !firrtl.uint<4>
reg _T_2621 : UInt<4>, clock with :
reset => (reset, UInt<4>("h0")) @[Edges.scala 230:27]

; CHECK: firrtl.div %i8, %34 : (!firrtl.uint<8>, !firrtl.uint<4>) -> !firrtl.uint<8>
; CHECK: firrtl.div %i8, %{{.*}} : (!firrtl.uint<8>, !firrtl.uint<4>) -> !firrtl.uint<8>
node n9 = div(i8, UInt<4>(4))

; CHECK: firrtl.tail %i8 7 : (!firrtl.uint<8>) -> !firrtl.uint<1>
Expand Down
12 changes: 6 additions & 6 deletions test/fir/locations.fir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ circuit basic : @[CIRCUIT.scala 127]
input in: UInt
output auto : { out_0 : UInt<1> }

; CHECK: %0 = firrtl.wire {name = "out_0"}
; CHECK: %out_0 = firrtl.wire {name = "out_0"}
wire out_0 : { member : { 0 : { reset : UInt<1>}}}

; %1 = firrtl.subfield %auto "out_0" : {{.*}} loc("Field":173:49)
; %2 = firrtl.subfield %0 "member" : {{.*}} loc("Field":173:49)
; %3 = firrtl.subfield %2 "0" : {{.*}} loc("Field":173:49)
; %4 = firrtl.subfield %3 "reset" : {{.*}} loc("Field":173:49)
; firrtl.partialconnect %1, %4 : {{.*}} loc("Field":173:49)
; %0 = firrtl.subfield %auto "out_0" : {{.*}} loc("Field":173:49)
; %1 = firrtl.subfield %out_0 "member" : {{.*}} loc("Field":173:49)
; %2 = firrtl.subfield %1 "0" : {{.*}} loc("Field":173:49)
; %3 = firrtl.subfield %2 "reset" : {{.*}} loc("Field":173:49)
; firrtl.partialconnect %0, %3 : {{.*}} loc("Field":173:49)
auto.out_0 <- out_0.member.0.reset @[Field 173:49]

; CIRCUIT: CHECK: } loc("CIRCUIT.scala":127:0)

0 comments on commit 0e95d97

Please sign in to comment.