Skip to content
This repository has been archived by the owner on Aug 20, 2024. It is now read-only.

Missing Constant Folders #2432

Open
7 tasks
seldridge opened this issue Dec 3, 2021 · 1 comment
Open
7 tasks

Missing Constant Folders #2432

seldridge opened this issue Dec 3, 2021 · 1 comment

Comments

@seldridge
Copy link
Member

seldridge commented Dec 3, 2021

The following constants are not folded when they likely should be:

  • asAsyncReset(const) -> const
  • asClock(const) -> const
  • cvt(const) -> signed(const)
  • mul(0, _) -> 0, mul(_, 0) -> 0, mul(0, 0) -> 0
  • div(0, _) -> 0
    • What is the precedence of div(0, _) vs. div(0, 0)? This may also match everyone's favorite controversial fold div(a, a) -> 1). 😉
  • rem(0, _) -> 0

To demonstrate any of these, you can construct a circuit like the following for asClock:

circuit Foo:
  module Foo :
    input clock : Clock
    input reset : UInt<1>
    input in : UInt<1>
    output out_0 : Clock
    output out_1 : Clock

    wire zero : UInt<1>
    zero <= UInt<1>(0)
    reg r_0 : Clock, clock with :
      reset => (UInt<1>("h0"), r_0)
    reg r_1 : Clock, clock with :
      reset => (UInt<1>("h0"), r_1)
    node _T = asClock(in)
    r_0 <= _T
    out_0 <= r_0
    node _T_1 = asClock(zero)
    r_1 <= _T_1
    out_1 <= r_1

If constant propagation succeeds, then register r_1 will be removed. If it doesn't, then it hangs around. E.g., the above produces:

  reg  r_0;
  reg  r_1;
  assign out_0 = r_0;
  assign out_1 = r_1;
  always @(posedge clock) begin
    r_0 <= in;
    r_1 <= 1'h0;
  end

It should produce:

  assign out_0 = r_0;
  assign out_1 = 1'h0;
  always @(posedge clock)
    r_0 <= in;

For a complete list of all tests, these are currently available in this branch: https://github.com/llvm/circt/blob/760d069d8ea7cac610f6ad22d962b1d9475458a8/test/Dialect/FIRRTL/SFCTests/invalid-reg-pass.fir. (These were generated using this script.) Anything that looks like a missed constant prop is marked with an <--. These use is invalid instead of constant zero, but it's the same thing during constant propagation in the Scala FIRRTL compiler as invalids are converted to constant zero.

Note that the asAsyncReset and asClock folds should likely happen, but the cleanest representation would be to use asyncreset and clock literals (which do not exist).

@sequencer
Copy link
Member

related to #1062

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants