by

Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Jack Koenig
    @jackkoenig
    @HCoffman I noticed that on the 1.3.x branch we only support IR nodes (eg. Reference and SubField) vs. WIR (eg. WRef and WSubField), have you run into this?
    This is no longer an issue on master where the WIR nodes have been folded into the IR nodes, but is an issue until 1.4 goes out
    Jack Koenig
    @jackkoenig
    *we only support IR nodes in the protobuf serialization
    Hunter Coffman
    @HCoffman
    I did run into the dshlw operator coming up during my testing but I was able to get around it easily. After a bit of trial and error, I was able to recreate it right now: ./utils/bin/firrtl -i ICache.lo.fir -X verilog --custom-transforms firrtl.transform.myWriteProtobuf.
    It's easy enough to ignore since I can just change -X verilog to -X none and get the protobuf, then just do the Verilog step after and that works fine
    Jack Koenig
    @jackkoenig
    A bit of a pain but glad you were able to workaround just fine
    matrixbot
    @matrixbot
    seldridge What're your prerequisites on your custom transform? I think you can hack around this by making sure that it runs before PadWidths.
    Hunter Coffman
    @HCoffman
    Since I'm specifically trying to work with LoFIRRTL right now, the only prerequisite I specify is that it's in that form. All my transform basically does is just do ToProto on a circuit and spit out the stream (not really much of a transform in all honesty).
    matrixbot
    @matrixbot
    seldridge So, inputForm = LowForm ?
    Hunter Coffman
    @HCoffman
    Yup!
    I don't really need any special hack on my end though since the only difference for me is whether I run one command or two when trying to get the Verilog and the Protobuf
    matrixbot
    @matrixbot
    seldridge Try something like the following (instead of using inputForm/outputForm:
    import firrtl.stage.Forms
    
    class myWriteProtobuf extends Transform with DependencyAPIMigration {
    
      override def prerequisites = Forms.LowForm
      override def optionalPrerequisites = Seq.empty
      override def optionalPrerequisiteOf = Forms.LowEmitters
      override def invalidates(a: Transform) = false
    
      override def execute = ???
    
    }
    seldridge There's some weirdness in the legacy way that inputForm=LowForm is scheduled. It roughly translates into "run as late as possible". So, if you run with -X low it runs right before the low firrtl emitter. If you run with -X verilog it runs after low FIRRTL optimizations (right before the Verilog emitter).
    seldridge Low FIRRTL optimizations introduce some illegal IR nodes that help with Verilog emission. It's a long-standing issue to clean this up. However, the newer dependency API lets you be more explicit about when things are supposed to run.
    Hunter Coffman
    @HCoffman
    Great, that worked! I was pretty curious about how everything was being scheduled so its pretty cool to hear about all this. Thanks again!
    matrixbot
    @matrixbot
    seldridge Fantastic. I'm glad to hear it.
    Boris V.Kuznetsov
    @tampler
    Started to add 2.13 support : freechipsproject/firrtl#1729 @azidar @jackkoenig @seldridge
    I also suggest to drop 2.11 support
    Jack Koenig
    @jackkoenig
    Awesome, thanks @tampler! I agree with dropping 2.11 support, but I think we should do it via deprecation schedule. Deprecate 2.11 in Chisel 3.4 / FIRRTL 1.4 (have the 2.11 artifacts print a warning at runtime). We can maybe add 2.13 during the 3.3.x release cycle to give those who want to upgrade a jumpstart. I want to make sure less Scala-savy users who are still on 2.11 because they don't know about upgrading have ample warning.
    Boris V.Kuznetsov
    @tampler
    Guys, what about banning infix expressions without the real need. Here's a code from FIRRTL sources:
    case "attach" => Attach(info, ctx_exp map visitExp )
    case "printf(" => Print(info, visitStringLit(ctx.StringLit), ctx_exp.drop(2).map(visitExp),
    Clearly, this was written by diff ppl in diff times. However, such "diversity" doesn't add to the code base stability. Sometimes may require extra compiler opts, like postfixOps.
    if its okay, I'll write a scalafmt script, which rewrites infix to standard notation, unless infix explicitly reqd
    Boris V.Kuznetsov
    @tampler
    Okay, finished the port. 2.11 and 2.12 seem to work, but for 2.13 have few test failed. @jackkoenig could you pls check CI results ? Any clues what went wrong there for 2.13 ?
    Boris V.Kuznetsov
    @tampler
    @jackkoenig Could you pls check what's wrong with the Unidoc in this build?
    Jack Koenig
    @jackkoenig
    @tampler There's been something wrong with the ANTLR/Protobuf plugins interfering with each other. Annoyingly I haven't been able to reproduce it locally
    Boris V.Kuznetsov
    @tampler
    This Unidoc thing is too old and outdated. I was thinking changing it to mdoc in the next MR
    Meanwhile, if you are okay with my changes, you should review and merge them . We'll update docs and CI on the following MRs
    Jack Koenig
    @jackkoenig
    Unidoc is for the ScalaDoc, how does mdoc fulfill that need?
    Boris V.Kuznetsov
    @tampler
    Oh, I see. mdoc is a typechecked markdown for documentation. I'll check how to config unidoc and other alternatives on next MRs
    Jack Koenig
    @jackkoenig
    The issue isn't Unidoc, it's the ANTLR and Protobuf SBT plugins
    Boris V.Kuznetsov
    @tampler
    This MR was meant to add 2.13 support and don't brake 2.11 and 2.12. That was done. 2.13 still needs to be added to CI
    Jack Koenig
    @jackkoenig
    There's some non-determinism in those SBT plugins, maybe bumping their versions would help, idk
    Boris V.Kuznetsov
    @tampler
    This CI uses Java 11.0.2 Oracle. I'm testing it on 11.0.7 OpenJDK
    Jack Koenig
    @jackkoenig
    Fortunately Java is stable enough that that shouldn't matter too much
    Boris V.Kuznetsov
    @tampler
    ANTLR and Unidoc seem to have the latest now
    Jack Koenig
    @jackkoenig
    We use GraalVM CE 20.0.0 (based on OpenJDK11)
    matrixbot
    @matrixbot
    Schuyler Eldridge I think these things are orthogonal. mdoc is good for running code blocks in markdown. (This is actually already how some of the website documentation works and it seems to work really well!) Scaladoc (or Unidoc) is still necessary as a means to document the underlying APIs.
    Jack Koenig
    @jackkoenig
    We do need to figure out what's up with the SBT plugins since they make CI flakey, but yeah it's just annoying
    Boris V.Kuznetsov
    @tampler
    Reverted to sbt 1.3.10. Let's see if CI passes with it
    Jack Koenig
    @jackkoenig
    It passed when I reran it, I think it's non-deterministic
    Oh wait maybe it didn't pass, well deterministic failure is better because at least we can fix it
    Boris V.Kuznetsov
    @tampler
    This may be an issue with CI. It may not download the deps on the first run. When you restart it, CI checks the download cache, picks deps and passes
    matrixbot
    @matrixbot
    Schuyler Eldridge That's the same behavior I've been seeing (twice with the nightly cron build). It passes when restarted.
    Boris V.Kuznetsov
    @tampler
    @jackkoenig You can fix it by disabling caching in CI. This way, it will restart download for each build. I'll need to check your CI script after that
    Jack Koenig
    @jackkoenig
    Rather than a back and forth of my editing stuff in Travis, it might be easier for you to just debug on a fork where you're an admin, should be identical
    Boris V.Kuznetsov
    @tampler
    @jackkoenig I don't have own CI for the cloned FIRRTL. It forwards me to freechips. Check this here
    Jack Koenig
    @jackkoenig
    If you're talking about the bade on the README, that's just a hyperlink. I opened a PR: chisel-crew/firrtl#1
    You have to enable the CI on both Github and Travis
    Jack Koenig
    @jackkoenig
    badge on the README*