chipsalliance / firrtl

Flexible Intermediate Representation for RTL
https://www.chisel-lang.org/firrtl/
Apache License 2.0
719 stars 176 forks source link

Breadcrumbs for all FIRRTL statements #679

Open stevenmburns opened 6 years ago

stevenmburns commented 6 years ago

It seems that most but not all FIRRTL statements have an informative "info" field. Some don't. High FIRRTL has many annotations, but some assignments aren't annotated with the source filename, line, and character position. For example _T_20 <= u in the code below:

       when _T_17 : @[BinaryGCD.scala 146:49]
          node _T_18 = shl(m, 1) @[BinaryGCD.scala 147:12]
          m <= _T_18 @[BinaryGCD.scala 147:7]
          skip @[BinaryGCD.scala 146:49]
        else : @[BinaryGCD.scala 148:16]
          wire _T_20 : UInt
          _T_20 is invalid
          _T_20 <= u
          wire _T_22 : UInt
          _T_22 is invalid
          _T_22 <= v
          node _T_23 = and(u, m) @[BinaryGCD.scala 134:47]
          node _T_25 = eq(_T_23, UInt<1>("h00")) @[BinaryGCD.scala 134:52]
          node _T_27 = eq(_T_25, UInt<1>("h00")) @[BinaryGCD.scala 152:12]

In low FIRRTL, some of the generated statements are unannotated (for example, the last seven lines in the following code)

    node _GEN_10 = mux(_T_39, _T_43, _GEN_4) @[BinaryGCD.scala 102:16:@46.4]
    node _GEN_11 = mux(_T_39, _GEN_9, _GEN_3) @[BinaryGCD.scala 102:16:@46.4]
    node _T_61 = dshl(u, k) @[BinaryGCD.scala 119:13:@79.4]
    node _T_62 = eq(u, v) @[BinaryGCD.scala 120:16:@81.4]
    node _T_64 = eq(v, UInt<1>("h0")) @[BinaryGCD.scala 120:27:@82.4]
    node _T_65 = or(_T_62, _T_64) @[BinaryGCD.scala 120:22:@83.4]
    io_z <= bits(_T_61, 63, 0)
    io_done <= _T_65
    u <= _GEN_11
    v <= _GEN_10
    k <= _GEN_5
    _T_41 <= _GEN_6
    _T_43 <= _GEN_7

How hard will it be to attach meaningful source code information to (all) the lines? It seems to be available in the node defining the temporary variable (for example, _GEN_10 above.)

I don't know if this is all of the missing cases, but fixing these two will be very helpful to me. I've built something that traces paths and displays source code so you can understand timing issues and now the connect statements don't point anywhere.

jackkoenig commented 6 years ago

There are a few places where source locators get destroyed, the primary one being ExpandWhens. This has been a known issue for a while but it seems we really ought to do something about it. People also have requested seeing the source locators as comments in the emitted Verilog.

There is a question of how to combine source locators, for example:

a <= foo @[Source.scala 17:8]
when cond : @[Source.scala 23:2]
  a <= bar @[Source.scala 37:3]

->

a <= mux(cond, bar, foo) @[Source.scala 17:8, Source.scala 23:2, Source.scala 37:3]

Perhaps we can do better by not repeating Source.scala, although these won't always be the same file obviously...

seldridge commented 6 years ago

There may be a related discussion here about whether the combination of Info should be a reversible operation.

chick commented 6 years ago

I'd like to add my name to the list of those wishing for this. I'd like treadle to be able to give source information in a number of places but it appears to be destroyed by the lowering passes