Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Change in behavior between 3.3.2 and 3.4.0-RC3 for naming #1603

Closed
jackkoenig opened this issue Sep 30, 2020 · 2 comments · Fixed by #1660
Closed

Change in behavior between 3.3.2 and 3.4.0-RC3 for naming #1603

jackkoenig opened this issue Sep 30, 2020 · 2 comments · Fixed by #1660

Comments

@jackkoenig
Copy link
Contributor

Type of issue: bug report

Impact: naming behavior change (back to old behavior)

Development Phase: request

Other information

If the current behavior is a bug, please provide the steps to reproduce the problem:

Given a somewhat weird (but useful) pattern:

import chisel3._
import chisel3.stage.ChiselStage

class Example(n: Int) extends MultiIOModule {
  val out = Seq.tabulate(n) { i =>
    val ioOpt = (if (i % 2 == 0) None else Some {
      val x = IO(Output(UInt(8.W)))
      x
    })
    ioOpt
  }
  
  out.foreach(_.foreach(_ := 0.U))
}

println((new ChiselStage).emitVerilog(new Example(4)))

The type of val out is Seq[Option[UInt]], this is actually very useful because the inner option allows us to skip indices in the namespace when there are Nones:

What is the current behavior?

In Chisel v3.4.0-RC3, this gives:

module Example(
  input        clock,
  input        reset,
  output [7:0] out_ioOpt_x,
  output [7:0] out_ioOpt_x_1
);
  assign out_ioOpt_x = 8'h0; // @[main.scala 15:27]
  assign out_ioOpt_x_1 = 8'h0; // @[main.scala 15:27]
endmodule

What is the expected behavior?

In Chisel v3.3.2, this gives:

module Example(
  input        clock,
  input        reset,
  output [7:0] out_1,
  output [7:0] out_3
);
  assign out_1 = 8'h0; // @[main.scala 15:27]
  assign out_3 = 8'h0; // @[main.scala 15:27]
endmodule

What is the use case for changing the behavior?

Restore previous (useful) naming behavior

@jackkoenig
Copy link
Contributor Author

@jackkoenig
Copy link
Contributor Author

jackkoenig commented Oct 22, 2020

This issue stems from the behavior of autoSeed on ports:
https://github.com/freechipsproject/chisel3/blob/26deb7703389b78a9b2a61f7e191f3f0e2a6623b/core/src/main/scala/chisel3/Data.scala#L289

Basically, ports have special behavior where the first seed wins. The "first seed wins" is nice because it fixes #1352. It's not really clear how we both fix this issue and #1352.

@mergify mergify bot closed this as completed in #1660 Nov 11, 2020
jackkoenig added a commit that referenced this issue Feb 28, 2023
* WIP Commit

* Add EdgeDataDiGraph with views to amortize graph construction

* WIP, got basic structure, need tests to pipeclean

* First tests pass. Need more.

* Tests pass, more need to be written

* More tests pass! Things should work, except for memories

* Added clearPrev to fix digraph uses where caching prev breaks

* Removed old Component. Documented IRLookup

* Added comments. Make prev arg to getEdges

* WIP: Refactoring for CircuitGraph

* Refactored into CircuitGraph. Can do topological module analysis

* Removed old versions

* Added support for memories

* Added cached test

* More stufffff

* Added implicit caching of connectivity

* Added tests for IRLookup, and others

* Many major changes.

Replaced CircuitGraph as ConnectionGraph
Added CircuitGraph to be top-level user-facing object
ConnectionGraph now automatically shortcuts getEdges
ConnectionGraph overwrites BFS as PriorityBFS
Added leafModule to Target
Added lookup by kind to IRLookup
Added more tests

* Reordered stuff in ConnectionGraph

* Made path work with deep hierarchies. Added PML for IllegalClockCrossings

* Made pathsInDAG work with current shortcut semantics

* Bugfix: check pathless targets when shortcutting paths

* Added documentation/licenses

* Removed UnnamedToken and related functionality

* Added documentation of ConnectionGraph

* Added back topo, needed for correct solving of intermediate modules

* Bugfix. Cache intermediate clockSources from same BFS with same root, but not BFS with different root

* Added literal/invalid clock source, and unknown top for getclocksource

* Bugfix for clocks in bundles

* Add CompleteTargetSerializer and test

* remove ClockFinder, be able to compile.

* test is able to compile, but need to fix.

* public and abstract DiGraph, remove DiGraphLike.

* revert some DiGraph code, ConnectionGraphSpec passed.

* CircuitGraphSpec passed.

* minimize diff between master

* codes clean up

* override linearize and revert DiGraph

* keep DiGraph unchanged.

* make ci happy again.

* codes clean up.

* bug fix for rebase

* remove wir

* make scaladoc happy again.

* update for review.

* add some documentation.

* remove tag

* wip IRLookup

* code clean up and add some doucmentations.

* IRLookup cache with ModuleTarget guarded.

* make unidoc and 2.13 happy

Co-authored-by: Adam Izraelevitz <azidar@gmail.com>
Co-authored-by: Albert Magyar <albert.magyar@gmail.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant