Add energy monitor #3

JiacongSun · 2025-10-07T16:50:06Z

Add energy monitor RTL and related submodule in a separate folder
Add related testbenches
Add the dtformat converter, the interface required by the analog island
Add dtformat converter testbench

Copilot

Pull Request Overview

This PR adds a comprehensive energy monitor module for hardware implementation along with related testbenches and format converters. The implementation includes an energy calculation system that computes Hamiltonian energy for spin-weight matrices with bias terms and scaling factors.

Adds energy monitor RTL module with pipeline support and control logic
Implements testbenches for all energy monitor components and submodules
Adds dtformat converter for 2's complement to signed magnitude conversion

Reviewed Changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
hw/unit_tests/partial_energy_calc/	Testbench and build files for partial energy calculation module
hw/unit_tests/energy_monitor/	Comprehensive testbenches for the main energy monitor module
hw/unit_tests/dtformat_converter/	Testbench for data format conversion module
hw/unit_tests/adder_tree/	Testbench for adder tree summation module
hw/unit_tests/accumulator/	Testbench for accumulator module
hw/rtl/lib/bp_pipe.sv	Backpressure pipeline module for configurable pipeline stages
hw/rtl/energy_monitor/	Core energy monitor RTL modules including control logic and computation units
hw/rtl/dtformat_converter.sv	Data format converter from 2's complement to signed magnitude
ci/ut-run.sh	Fix for GUI flag logic in test runner script

Comments suppressed due to low confidence (2)

hw/unit_tests/energy_monitor/README.md:1

Corrected spelling of 'Incrimental' to 'Incremental'.

# Energy Monitor Testbench

hw/unit_tests/accumulator/tb_accumulator.sv:1

Overflow detection logic is incorrect. Line 52 compares sign bits incorrectly - should compare accum_reg[ACCUM_WIDTH-1] == data_i[IN_WIDTH-1] since data_i has IN_WIDTH bits, not ACCUM_WIDTH.

// Copyright 2025 KU Leuven.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

hw/unit_tests/partial_energy_calc/tb_partial_energy_calc.sv

hw/unit_tests/energy_monitor/tb_energy_monitor_pipes.sv

hw/unit_tests/energy_monitor/tb_energy_monitor.sv

hw/rtl/energy_monitor/step_counter.sv

hw/rtl/energy_monitor/partial_energy_calc.sv

hw/rtl/energy_monitor/accumulator.sv

ci/ut-run.sh

hw/rtl/dtformat_converter.sv

clmcsn

Hi Jiacong,

before approval i would like you to:

Clarify in the RTL files under which cases they have been tested (both parameters setting and input stimuli) as a warning for future reuse.
Trim and remove unused lines and unnecessary files (see comments)
Add source files to the Bender.yml
Add energy monitor test to the ci pipeline every time we push to github. See if it is possible to trigger the test only if we change the files related to the block.

Thank youuu! Very good job!

hw/rtl/dtformat_converter.sv

hw/rtl/energy_monitor/accumulator.sv

hw/rtl/energy_monitor/logic_ctrl.sv

clmcsn · 2025-10-08T10:41:22Z

hw/rtl/energy_monitor/vector_mux.sv

@@ -0,0 +1,25 @@
+// Copyright 2025 KU Leuven.


This module is too small: 1 assign block! (up to 10-15 lines, you should always put them inline, unless those 15 lines are a common block that is reused somewhere else)
You should just put it in the top module and comment with e.g. //bit selection mux logic

The top module better not mix with the data processing part, right? It should keep a clear view of the architecture. Otherwise, it does not appear well-structured in terms of hierarchy.

I don't agree. You can have a look at bigger and more mature designs (from CVA6 core to ibex that are at industry standard level).

You put something in another file only if it is a major block or if it is a library component, but not a single assign line.

Nevertheless, you can keep it like this as you are the designer and it is fair to be your choice

clmcsn · 2025-10-08T11:05:54Z

hw/unit_tests/accumulator/tb_accumulator.sv

+    // Accumulation test
+    // use random data, random valid signal and random enable signal
+    initial begin
+        wait (clear_test_done == 1);


A bit hacky! You might want to use tasks for future tb, they are the same thing but you can call in one single initial block

I will keep it simple as this is a test for a leaf module.

clmcsn · 2025-10-08T11:07:38Z

hw/unit_tests/adder_tree/tb_adder_tree.sv

+    // Testcases
+    // Test patterns for adder tree
+    logic signed [N*DATAW-1:0] test_a[NUM_TESTS] = '{
+        {N{8'sd0}}, // All zeros


Not the most extensive test, but OK for now. We can work on a more practical and automated way to test blocks. E.g., use some python scripts to generate random input vectors

I will keep it simple as this is a test for a leaf module.

clmcsn · 2025-10-08T11:13:51Z

hw/unit_tests/adder_tree/tb_adder_tree.sv

+
+module tb_adder_tree;
+
+    localparam int N = 256; // number of inputs


What if we have a different configuration?
Maybe add to the RTL blocks under which cases they have been tested (so we don't reuse them in other cases :))

I will keep it simple as this is a test for a leaf module.

clmcsn · 2025-10-08T11:28:55Z

hw/unit_tests/energy_monitor/tb_energy_monitor.sv

@@ -0,0 +1,363 @@
+// Copyright 2025 KU Leuven.


The testing procedure should be a bit more structured:

Smoke test (simple input)

Corner case (trigger special behavior)

Random heavy testing (100 is not enough, you should go for 1 million!)

Each test should be contained in a separate file and easily inspectable by eye to verify its solidity.
This should also be the case for the correctness check routines. I would finally write this in even another language (e.g., Python) to ensure even more that we are doing something that makes sense.
I am not asking to change it, but I would like to start a better way to test that will make our lives easier as well (tb are a pain)

Good advice. To be done after finishing the entire RTL.

JiacongSun · 2025-10-13T12:13:59Z

In the current version, energy_monitor's state machine is updated so that the weight fetching and computation happen in parallel. Now for #spins=256, it takes 256+1 cycles (extra one for spin fetching).

The tb has been fixed and a random test with 1_000_000 test cases have been verified successfully.

jiacong sun added 30 commits September 15, 2025 16:25

add backpresure pipe module

ce37be9

create inferface of energy monitor

e04bdd8

add adder_tree and partial_energy_calc modules

33e9636

add adder_tree

a56f028

add partial_energy_calc module

a92ac7a

change the h multiplier to shift operation

8b8afd8

remove the 2c-to-sm part

901cbd0

add dtformat_converter module

b77b9e7

fix a typo in ci/ut-run.sh

53c6a74

fix an incorrect signed interprectation for the adder tree

814f133

fix the sign extension issue for the energy calc module

21ac248

fix the sign extension issue for the energy calc module

092496d

add accumulator

b817457

remove incorrect comment

2d6de19

complete the accumulator

8d4d841

update hdl file list for accumulator

37395de

update accumulator module and initial version of testbench

e55f47d

remove obsolete parameter SPINIDX_BIT

1fe2851

update interface definition

773d1e3

remove sign-magnitude to 2s conversion

f08c8ef

remove the part related to spin mask

93538a6

update the logic to use current_spin without spin mask

5edc404

remove en_i from the interface

2905cdf

update the path of registers.svh

fa5b3c9

remove the clean_i from the ctrl path

b2d0ca1

change the parameter name for output energy precision

f74311f

add counter ctrl module

cbaeddd

add vector mux module

317512c

add the step counter module

c86887a

add the logic fsm

2010bdc

jiacong sun added 10 commits September 30, 2025 21:36

add spin caching module into file list

2c28dd3

fix the energy monitor tb without pipes

81bcc5a

add readme to energy monitor tb

ed0b3c0

correct a typo in the readme

0f34bab

Merge branch 'main' into energy_monitor

3fa4b2c

add module interface and register table to readme

5de344a

add module interface and register table to readme

6096798

update readme

248fd27

add tb for energy monitor with pipes

d30de9f

create separate folder for energy-monitor related modules

3d96bb4

JiacongSun requested review from clmcsn and Copilot October 7, 2025 16:50

JiacongSun self-assigned this Oct 7, 2025

Copilot AI reviewed Oct 7, 2025

View reviewed changes

clmcsn reviewed Oct 8, 2025

View reviewed changes

hw/rtl/dtformat_converter.sv Outdated Show resolved Hide resolved

clmcsn requested changes Oct 8, 2025

View reviewed changes

jiacong sun added 13 commits October 8, 2025 15:34

fix the port mismatch in tb for partial_energy_calc

50c4474

remove the incorrect sign declaration in tb_partial_energy_calc

656eaae

fix the typo

cdddff4

rename spin_i to spin_vector_i

9e84e4c

correct the bandwidth of data_i

88c858e

remove unused lines

ce06cf5

correct the file path

75e381d

rename _next to _n

8e61bb5

move comments to the top; add testcases to rtl

dbdecad

switch to use FFLARNC in accumulator

beb1333

update the logic ctrl; to be fixed

d9592dc

remove unsed lines

785bd64

fix the energy monitor state machine and tb

8caf8bd


		module tb_adder_tree;

		localparam int N = 256; // number of inputs

Add energy monitor #3

Are you sure you want to change the base?

Add energy monitor #3

Uh oh!

Conversation

JiacongSun commented Oct 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clmcsn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JiacongSun commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants