amaranth icon indicating copy to clipboard operation
amaranth copied to clipboard

Translate ArrayProxy to pattern matching when supported

Open Jimx- opened this issue 3 years ago • 4 comments

Current the value compiler translates ArrayProxy into if-elif trees which can cause the compiler to crash due to deep recursion (#359). This patch instead translates them into pattern matching when it is supported (Python >= 3.10) to avoid this problem.

Jimx- avatar Sep 13 '22 06:09 Jimx-

This is an interesting solution--I'll need to look at it a bit closer.

What's performance like?

whitequark avatar Sep 13 '22 08:09 whitequark

Pattern matching seems to be slightly faster than if-elif for compiling

m = Module()
mem = Memory(width=32, depth=0xb80)
m.submodules.rport = mem.read_port()
m.submodules.wport = mem.write_port()
sim = Simulator(m)
sim.run()

Pattern matching: python test.py 0.28s user 0.02s system 99% cpu 0.299 total

if-elif: python test.py 0.36s user 0.02s system 99% cpu 0.380 total

If-elif crashes for depth over 0xc00 with the default recursion limit

Patter matching with depth 0x4000: python test.py 1.48s user 0.10s system 99% cpu 1.581 total

Jimx- avatar Sep 13 '22 11:09 Jimx-

What about runtime performance?

whitequark avatar Sep 13 '22 11:09 whitequark

They are very close

    def process():
        for i in range(depth):
            yield dut.waddr.eq(i)
            yield dut.wdata.eq(i)
            yield dut.we.eq(1)
            yield

    sim.add_sync_process(process)
    start = time.time()
    sim.run()
    end = time.time()
    print(end - start)

It took both of them about 1.5s to finish

Jimx- avatar Sep 13 '22 13:09 Jimx-

Could you rebase on top of main, please? The CI should pass afterwards.

whitequark avatar Sep 24 '22 06:09 whitequark

Codecov Report

Merging #712 (a3546bd) into main (c4be739) will increase coverage by 0.06%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #712      +/-   ##
==========================================
+ Coverage   81.10%   81.17%   +0.06%     
==========================================
  Files          49       49              
  Lines        6511     6533      +22     
  Branches     1754     1764      +10     
==========================================
+ Hits         5281     5303      +22     
  Misses       1036     1036              
  Partials      194      194              
Impacted Files Coverage Δ
amaranth/sim/_pyrtl.py 95.58% <100.00%> (+0.28%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Sep 24 '22 10:09 codecov-commenter

Thank you!

whitequark avatar Sep 24 '22 10:09 whitequark