Some statistics are zero, disabling threading seems to fix

[dtzSiFive]

Noticed this while adding statistics, here's a small test case names-cse.fir:

circuit Names:
  module Names:
    input i: UInt<1>
    input j: UInt<1>
    output o: UInt
    wire w: UInt
    wire w2: UInt
    w <- and(i, j)
    w2 <- and(i, j)
    o <- add(w, w2)

This can be observed with --mlir-pass-statistics, for example:

$ ./bin/firtool --mlir-pass-statistics ./names-cse.fir |& grep '(S)'
    (S) 0 num-added-annos     - Number of additional annotations
    (S) 0 num-annos           - Total number of annotations processed
    (S) 0 num-unhandled-annos - Number of unhandled annotations
    (S) 0 num-raw-annos       - Number of raw annotations on circuit
      (S) 0 num-names-converted - Number of interesting names made droppable
      (S) 0 num-dce'd - Number of operations DCE'd
      (S) 0 num-cse'd - Number of operations CSE'd
    (S) 0 num-erased-op - Number of operations erased
    (S) 0 num-folded-op - Number of operations folded
      (S) 0 num-names-converted - Number of interesting names made droppable
    (S) 0 num-removed-ports - Number of ports erased
    (S) 0 num-dce'd - Number of operations DCE'd
    (S) 0 num-cse'd - Number of operations CSE'd
    (S) 0 num-dce'd - Number of operations DCE'd
    (S) 2 num-cse'd - Number of operations CSE'd

Note this reports no names were dropped/converted, all statistics are zero except the last CSE statistic (?).

Compared to disabling threads:

$ ./bin/firtool --mlir-pass-statistics --mlir-disable-threading ./names-cse.fir |& grep '(S)'
    (S) 0 num-added-annos     - Number of additional annotations
    (S) 0 num-annos           - Total number of annotations processed
    (S) 0 num-unhandled-annos - Number of unhandled annotations
    (S) 0 num-raw-annos       - Number of raw annotations on circuit
      (S) 2 num-names-converted - Number of interesting names made droppable
      (S) 0 num-dce'd - Number of operations DCE'd
      (S) 2 num-cse'd - Number of operations CSE'd
    (S) 0 num-erased-op - Number of operations erased
    (S) 0 num-folded-op - Number of operations folded
      (S) 2 num-names-converted - Number of interesting names made droppable
    (S) 0 num-removed-ports - Number of ports erased
    (S) 0 num-dce'd - Number of operations DCE'd
    (S) 0 num-cse'd - Number of operations CSE'd
    (S) 0 num-dce'd - Number of operations DCE'd
    (S) 2 num-cse'd - Number of operations CSE'd

Where you can see 2 names are converted, and an earlier CSE run is non-zero also.

Passes impacted are firrtl.modue passes, but I'm not sure if that's all that's needed.

[dtzSiFive]

FWIW a total of 2 names are dropped, so depending how this is interpreted they both are kinda right on that front.

That said, when using the list display (-mlir-pass-statistics-display=list) it prints the statistics once per pass and shows 0 names dropped:

  AddSeqMemPorts
    (S) 0 num-added-ports - Number of extra ports added
  CSE
    (S) 2 num-cse'd - Number of operations CSE'd
    (S) 0 num-dce'd - Number of operations DCE'd
  DropName
    (S) 0 num-names-converted - Number of interesting names made droppable
  FlattenMemory
    (S) 0 num-flatten-mems - Number of memories flattened
  GrandCentral
    (S) 0 num-annotations-removed - Number of annotations removed
    (S) 0 num-interfaces-created  - Number of SystemVerilog interfaces that were created
    (S) 0 num-views-created       - Number of top-level SystemVerilog interfaces that were created
    (S) 0 num-xmrs-created        - Number of SystemVerilog XMRs added
  GrandCentralSignalMappings
    (S) 0 num-buffer-wire-pairs-added - Number of buffer wire pairs added to break nets around forced ports
    (S) 0 num-forced-input-ports      - Number of input ports that will be forced (sinks)
    (S) 0 num-forced-output-ports     - Number of output ports that will be forced (sinks)
    (S) 0 num-modules                 - Number of modules with at least one source or sink
    (S) 0 num-sinks                   - Number of sinks
    (S) 0 num-sources                 - Number of sources
  IMConstProp
    (S) 0 num-erased-op - Number of operations erased
    (S) 0 num-folded-op - Number of operations folded
  IMDeadCodeElim
    (S) 0 num-erased-modules - Number of modules erased
    (S) 0 num-erased-ops     - Number of operations erased
    (S) 0 num-removed-ports  - Number of ports erased
  InferReadWrite
    (S) 0 num-rw-port-mems-inferred - Number of memories inferred to use RW port
  InnerSymbolDCE
    (S) 0 num-sym-found   - Number of symbols found
    (S) 0 num-sym-removed - Number of symbols removed
  LowerCHIRRTLPass
    (S) 0 num-created-mems  - Number of memories created
    (S) 0 num-lowered-mems  - Number of memories lowered
    (S) 0 num-portless-mems - Number of memories dropped as having no valid ports
  LowerFIRRTLAnnotations
    (S) 0 num-added-annos     - Number of additional annotations
    (S) 0 num-annos           - Total number of annotations processed
    (S) 0 num-raw-annos       - Number of raw annotations on circuit
    (S) 0 num-reused-hierpath - Number of reused HierPathOp's
    (S) 0 num-unhandled-annos - Number of unhandled annotations
  MemToRegOfVec
    (S) 0 num-converted-mems - Number of memories converted to registers
  SymbolDCE
    (S) 0 num-dce'd - Number of symbols DCE'd

Note that we now run CSE 5 times instead of 2 when issue was first reported.

FWIW upstream there's an issue about dynamic pipeline statistics being dropped (https://llvm.org/PR57622) which we do not use in a significant way presently.

There's code to merge statistics from async passmanagers into "main" pass manager: https://github.com/llvm/llvm-project/blob/47c68904a53a5aeeb2f7d506ac6be7d1a0fb951e/mlir/lib/Pass/PassStatistics.cpp#L228 , which is likely where this is going wrong / occurring, at least a good place to look when investigating this further.

[dtzSiFive]

Upstream issue, fix submitted and patch approved (https://reviews.llvm.org/D139459). Should land in time for this week's LLVM bump.

[dtzSiFive]

Fix landed! Should be resolved with next LLVM bump.

[dtzSiFive]

Fixed with merge of #4402 !