{"action":"created","issue":{"url":"https://api.github.com/repos/ucb-bar/chisel3/issues/293","repository_url":"https://api.github.com/repos/ucb-bar/chisel3","labels_url":"https://api.github.com/repos/ucb-bar/chisel3/issues/293/labels{/name}","comments_url":"https://api.github.com/repos/ucb-bar/chisel3/issues/293/comments","events_url":"https://api.github.com/repos/ucb-bar/chisel3/issues/293/events","html_url":"https://github.com/ucb-bar/chisel3/pull/293","id":178792742,"number":293,"title":"Add support for parameterized BlackBoxes","user":{"login":"jackkoenig","id":8584418,"avatar_url":"https://avatars.githubusercontent.com/u/8584418?v=3","gravatar_id":"","url":"https://api.github.com/users/jackkoenig","html_url":"https://github.com/jackkoenig","followers_url":"https://api.github.com/users/jackkoenig/followers","following_url":"https://api.github.com/users/jackkoenig/following{/other_user}","gists_url":"https://api.github.com/users/jackkoenig/gists{/gist_id}","starred_url":"https://api.github.com/users/jackkoenig/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jackkoenig/subscriptions","organizations_url":"https://api.github.com/users/jackkoenig/orgs","repos_url":"https://api.github.com/users/jackkoenig/repos","events_url":"https://api.github.com/users/jackkoenig/events{/privacy}","received_events_url":"https://api.github.com/users/jackkoenig/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2016-09-23T06:07:37Z","updated_at":"2016-09-23T21:26:18Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ucb-bar/chisel3/pulls/293","html_url":"https://github.com/ucb-bar/chisel3/pull/293","diff_url":"https://github.com/ucb-bar/chisel3/pull/293.diff","patch_url":"https://github.com/ucb-bar/chisel3/pull/293.patch"},"body":"Also restrict black boxes to not allow hardware inside of them since it was\r\nbeing silently dropped anyway.\r\n\r\nResolves #289 \r\n\r\nThis PR obviously depends on ucb-bar/firrtl#314 in order to pass tests. Also, thanks to the new Firrtl feature where extmodules can overwrite their name, Chisel emits BlackBoxes that will always get their desired names.\r\n\r\nThe current API proposed by this PR is to make blackbox parameters be an argument to the BlackBox abstract class of type Map[String, Any] where Any is dynamically checked to be only be Int, Long, BigInt, Double, or String. This leads to the use in one of the test cases of:\r\n\r\n```scala\r\nclass BlackBoxConstant(params: Map[String, Any]) extends BlackBox(params) {\r\n  val io = new Bundle {\r\n    val out = UInt(width = params(\"WIDTH\").asInstanceOf[Int]).asOutput\r\n  }\r\n}\r\n\r\nclass BlackBoxWithParamsTester extends BasicTester {\r\n  val blackBoxOne  = Module(new BlackBoxConstant(Map(\"WIDTH\" -> 1, \"VALUE\" -> 1)))\r\n  val blackBoxFour  = Module(new BlackBoxConstant(Map(\"WIDTH\" -> 3, \"VALUE\" -> 4)))\r\n  ...\r\n}\r\n```\r\n\r\nAlternatively, params could be a def overridden in the BlackBox body itself, then users could pass their parameters however they see fit and transform them to the Map[String, Any] in the body of the BlackBox.\r\n\r\nI'm not the biggest fan of the above syntax and am certainly open to suggestions."},"comment":{"url":"https://api.github.com/repos/ucb-bar/chisel3/issues/comments/249306448","html_url":"https://github.com/ucb-bar/chisel3/pull/293#issuecomment-249306448","issue_url":"https://api.github.com/repos/ucb-bar/chisel3/issues/293","id":249306448,"user":{"login":"aswaterman","id":1031106,"avatar_url":"https://avatars.githubusercontent.com/u/1031106?v=3","gravatar_id":"","url":"https://api.github.com/users/aswaterman","html_url":"https://github.com/aswaterman","followers_url":"https://api.github.com/users/aswaterman/followers","following_url":"https://api.github.com/users/aswaterman/following{/other_user}","gists_url":"https://api.github.com/users/aswaterman/gists{/gist_id}","starred_url":"https://api.github.com/users/aswaterman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aswaterman/subscriptions","organizations_url":"https://api.github.com/users/aswaterman/orgs","repos_url":"https://api.github.com/users/aswaterman/repos","events_url":"https://api.github.com/users/aswaterman/events{/privacy}","received_events_url":"https://api.github.com/users/aswaterman/received_events","type":"User","site_admin":false},"created_at":"2016-09-23T21:26:18Z","updated_at":"2016-09-23T21:26:18Z","body":"If I'm parsing this thread right, @sdtwigg's opinion is about purity of style, and so is mostly a distraction from the usability discussion at hand.  OTOH, I think @mwachs5's argument stems from the notion that parameters, like io, are part of a module's interface; their types/names should be part of the module declaration, whereas their values should be bound at instantiation time.  @jackkoenig's second proposal partially achieves that goal, but doesn't illustrate the opportunity for type safety, and doesn't illustrate that it can be done with DRY.\r\n\r\nI still favor the approach of passing args to the BlackBox constructor.  I think it's akin to parameterizing a Module on type.  Consider Queue.  Ignoring the Scala type system for a thought experiment, you could consider making it an abstract class, which you make concrete with \"def tpe = foo.\"  That would achieve the same thing as passing tpe to the Queue constructor, but seems more awkward.\r\n\r\nSimilarly, it's not uncommon to parameterize Module based on an integer parameter, like the width of a signal.  To me, it's most natural to pass that as an arg to the constructor."}}
{"actor":{"display_login":"aswaterman"}}