chipsalliance / firrtl-spec

The specification for the FIRRTL language
38 stars 27 forks source link

[patch] Clarify existing when behavior #215

Closed seldridge closed 2 months ago

seldridge commented 2 months ago

Add language that defines what conditionals are (they are a region where the operations in the region change their behavior based on the condition) and how different operations are affected by the condition. This fills a hole where the behavior of commands was not explicitly specified. Add an aside that clarifies that this means that a trivial inlining will not work as a user might expect.

mmaloney-sf commented 2 months ago

Also, I realized I missed the implication that "trivial inlining is not possible."

What's an example of this? (Just in the comments here is fine).

I can imagine inlining a module definition's body wouldn't work. But I just chalk that up to declarations-under-whens is just a bad idea.

seldridge commented 2 months ago

What's an example of this? (Just in the comments here is fine).

Wires and registers are fine. However, any command is not. Consider:

FIRRTL version 4.0.0
circuit Foo:
  module Bar:
    input clock: Clock
    printf(clock, UInt<1>, "hello")
  public module Foo:
    input clock: Clock
    input cond: UInt<1>

    when cond:
      inst bar of Bar
      connect bar.clock, clock

If this is trivially inlined, you get:

FIRRTL version 4.0.0
circuit Foo:
  public module Foo:
    input clock: Clock
    input cond: UInt<1>

    when cond:
      printf(clock, UInt<1>, "hello")

The problem is that before the trivial inlining, the printf was unconditional. However, after the trivial inlining, then the printf is conditional.

This doesn't seem really problematic. Instead, it just means that inlining (or outlining) needs to be handled correctly and the above trivial inlining is not correct.