llvm / circt

Circuit IR Compilers and Tools
https://circt.org
Other
1.67k stars 298 forks source link

CSE Doesn't Merge Source Locators #4358

Open seldridge opened 1 year ago

seldridge commented 1 year ago

I've noticed that CSE doesn't seem to merge source locators. E.g., in the following, the source locator on the second add gets dropped:

circuit Foo:
  module Foo:
    input a: UInt<1>
    input b: UInt<1>

    node x = add(a, b)
    node y = add(a, b)

Running with circt-translate -import-firrtl cse.fir | circt-opt -cse -mlir-print-debuginfo -mlir-print-ir-after-all -mlir-print-ir-before-all produces:

// -----// IR Dump Before CSE (cse) //----- //
module {
  firrtl.circuit "Foo"  {
    firrtl.module @Foo(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>) {
      %0 = firrtl.add %a, %b : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<2> loc(#loc3)
      %x = firrtl.node interesting_name %0  : !firrtl.uint<2> loc(#loc4)
      %1 = firrtl.add %a, %b : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<2> loc(#loc5)
      %y = firrtl.node interesting_name %1  : !firrtl.uint<2> loc(#loc6)
    } loc(#loc2)
  } loc(#loc1)
} loc(#loc)
#loc = loc("<stdin>":1:1)
#loc1 = loc("<stdin>":2:3)
#loc2 = loc("<stdin>":3:5)
#loc3 = loc("<stdin>":4:12)
#loc4 = loc("<stdin>":5:12)
#loc5 = loc("<stdin>":6:12)
#loc6 = loc("<stdin>":7:12)

// -----// IR Dump After CSE (cse) //----- //
module {
  firrtl.circuit "Foo"  {
    firrtl.module @Foo(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>) {
      %0 = firrtl.add %a, %b : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<2> loc(#loc3)
      %x = firrtl.node interesting_name %0  : !firrtl.uint<2> loc(#loc4)
      %y = firrtl.node interesting_name %0  : !firrtl.uint<2> loc(#loc5)
    } loc(#loc2)
  } loc(#loc1)
} loc(#loc)
#loc = loc("<stdin>":1:1)
#loc1 = loc("<stdin>":2:3)
#loc2 = loc("<stdin>":3:5)
#loc3 = loc("<stdin>":4:12)
#loc4 = loc("<stdin>":5:12)
#loc5 = loc("<stdin>":7:12)

module {
  firrtl.circuit "Foo"  {
    firrtl.module @Foo(in %a: !firrtl.uint<1>, in %b: !firrtl.uint<1>) {
      %0 = firrtl.add %a, %b : (!firrtl.uint<1>, !firrtl.uint<1>) -> !firrtl.uint<2> loc(#loc3)
      %x = firrtl.node interesting_name %0  : !firrtl.uint<2> loc(#loc4)
      %y = firrtl.node interesting_name %0  : !firrtl.uint<2> loc(#loc5)
    } loc(#loc2)
  } loc(#loc1)
} loc(#loc)
#loc = loc("<stdin>":1:1)
#loc1 = loc("<stdin>":2:3)
#loc2 = loc("<stdin>":3:5)
#loc3 = loc("<stdin>":4:12)
#loc4 = loc("<stdin>":5:12)
#loc5 = loc("<stdin>":7:12)

I would expect to see the original #loc5 (:6:12) to be merged with #loc3, but it instead is dropped. This may be an MLIR limitation.

dtzSiFive commented 1 year ago

I would expect to see the original #loc5 (:6:12) to be merged with #loc3, but it instead is dropped. This may be an MLIR limitation.

Looks like you're right, CSE will not fuse locations, but will keep the location of the replaced op if the original does not have a location ("unknown"):

https://github.com/llvm/llvm-project/blob/22d87b82124f9c0de87362282e78b4835518221a/mlir/lib/Transforms/CSE.cpp#L200