llvm / llvm/circt

[FIRRTL] Don't Dedup Modules with Different NestedPrefixModulesAnnotation

Open
#2,888 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

NestedPrefixModulesAnnotation with different prefixes should not deduplicate. This can be interpreted one of two ways, either of which should have the same effect:

  1. NestedPrefixModulesAnnotation should block deduplication.
  2. The PrefixModules pass should duplicate modules which have different prefixes.

SiFive's extensions to the SFC are implementing this using strategy (2).

An end-to-end test case that should work is the following:

circuit Foo: %[[
  {
    "class":"sifive.enterprise.firrtl.NestedPrefixModulesAnnotation",
    "prefix":"prefix1_",
    "inclusive":true,
    "target":"~Foo|Bar"
  },
  {
    "class":"sifive.enterprise.firrtl.NestedPrefixModulesAnnotation",
    "prefix":"prefix2_",
    "inclusive":true,
    "target":"~Foo|Baz"
  }
]]
  module Bar:
  module Baz:
  module Foo:
    inst bar of Bar
    inst baz of Baz

Currently, this produces (firtool -dedup -strip-debug-info):

module prefix2_Bar();
endmodule

module Foo();
  prefix2_Bar bar ();
  prefix2_Bar baz ();
endmodule

This should produce:

module prefix1_Bar();
endmodule

module prefix2_Baz();
endmodule

module Foo();
  prefix1_Bar bar ();
  prefix2_Baz baz ();
endmodule

There may also be an interaction here with memories where memories should not deduplicate if they have different prefixes. This may be a little funky because it has interactions with the memory generator schema which may not respect prefixing.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the provided FIRRTL circuit with firtool -dedup -strip-debug-info, then read the PrefixModules pass and the deduplication path it interacts with. Check the possible memory-generator interaction described in the issue. Done means the example preserves prefix1_Bar and prefix2_Baz as separate modules with the corresponding instances, with coverage for the differing prefixes.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.