LZA (Leading zeros anticipation) #741

Durchbruchswagen · 2024-10-29T09:39:45Z

Leading Zero anticipation module implementation for FPU adder-subtractor based on: https://userpages.cs.umbc.edu/phatak/645/supl/lza/lza-survey-arith01.pdf

piotro888 · 2024-11-10T12:09:11Z

coreblocks/func_blocks/fu/fpu/lza.py

+        m = TModule()
+
+        @def_method(m, self.predict_request)
+        def _(arg):


Suggested change

def _(arg):

def _(sig_a, sig_b, carry):

"new" argument syntax is preferred for small number of arguments

lekcyjna123

Looks pretty well, but I left some comments.

lekcyjna123 · 2024-11-10T12:17:32Z

coreblocks/func_blocks/fu/fpu/lza.py

+
+
+class LZAModule(Elaboratable):
+    """LZA module


Please extend the docstring. Let assume, that reader doesn't know anything about floating point operation. Will he know what is the goal of that module?

Additionaly add a link to the paper, which you attached to the commit msg.

lekcyjna123 · 2024-11-10T12:20:36Z

coreblocks/func_blocks/fu/fpu/lza.py

+    """
+
+    def __init__(self, *, fpu_params: FPUParams):
+        self.predict_in_layout = [


What is the meaning of each subfield?

Done, added comment explaining each subfield

lekcyjna123 · 2024-11-10T12:25:19Z

coreblocks/func_blocks/fu/fpu/lza.py

+                m.d.av_comb += z[0].eq(1)
+
+            for i in reversed(range(1, self.lza_params.sig_width + 1)):
+                m.d.av_comb += f[i - 1].eq((t[i] ^ ~(z[i - 1])))


Here is the only usage of z, so you have ~(~A & ~B). I think that this can be simplified and z can be defined as A | B.

You are right, we can define z as (sig_a | sig_b) plus some operation for z[0] depending on carry

lekcyjna123 · 2024-11-10T12:29:33Z

coreblocks/func_blocks/fu/fpu/lza.py

+                m.d.av_comb += z[0].eq(1)
+
+            for i in reversed(range(1, self.lza_params.sig_width + 1)):
+                m.d.av_comb += f[i - 1].eq((t[i] ^ ~(z[i - 1])))


Is the shift by one in f intentional? In paper you have equation (2) f[i] = t[i] ^ ~z[i+1] which translates to our indexing: f[i] = t[i] ^ ~z[i-1]

Yes, it is intentional. g, z, t, has a width of sig_width + 1 (to account for carry, which allows us to predict either a+b or a+b+1 depending on its value), while f has a width of sig_width. So basically i for z, t or g maps to i-1 for f.

lekcyjna123 · 2024-11-10T12:32:19Z

coreblocks/func_blocks/fu/fpu/lza.py

+                m.d.av_comb += f[i - 1].eq((t[i] ^ ~(z[i - 1])))
+
+            m.d.av_comb += shift_amount.eq(0)
+            for i in reversed(range(self.lza_params.sig_width)):


Maybe it is better to use count_leading_zeros from amaranth_ext? It has logarithmic critical path and you have linear.

The problem with count_leading_zeroes is that it requires a number to have a width that is an exact power of 2. Significands usually do not fulfill this requirement. I would have to extend input widths to the nearest power of 2 and do some operations to convert shift for that number to shift for our target width. I could also do it by creating a string L out of f that only has 1 on the postion of left-most 1 in f and then using count_trailing_zeros, but I don't know if this is worth it.

Yes, I thought about extending count_leading_zeros with 1s to the nearest power of two on LSBs. This will cause that number of zeros returned by count_leading_zeros for the extended number will be the same as for non-extended, so no additional processing will be needed.

Done, extended f to nearest power of two and filled lower bits with 1

lekcyjna123 · 2024-11-10T12:36:18Z

test/func_blocks/fu/test_lza.py

+            self.test_val_sig_a_6 = 8421376
+            self.test_val_sig_b_6 = 8421376
+
+    def test_manual(self):


Maybe add also a random test?

sig_a = randomint() sig_b = randomint() pred_lz = lza(sig_a, sig_b) true_lz = count_leading_zero(sig_a+sig_b) assert pred_lz == true_lz or pred_lz == true_lz + 1

Done. I had to do some more work to ensure that numbers are normalized and a>=b, but overall the function random_test looks more or less the same.

lekcyjna123 · 2024-11-16T12:24:01Z

test/func_blocks/fu/test_lza.py

@@ -37,6 +38,27 @@ def test_manual(self):
        help_values = TestLZA.HelpValues(params)
        lza = TestLZA.LZAModuleTest(params)

+        def clz(sig_a, sig_b, carry):


This can be a generic function in test framework to be used also in other tests.

lekcyjna123 · 2024-11-16T12:25:56Z

test/func_blocks/fu/test_lza.py

@@ -121,6 +143,7 @@ def lza_test():

        def test_process():
            yield from lza_test()
+            yield from random_test()


Usually when you do random test you want to set a seed and execute more than 1 iteration.

tilk · 2024-11-19T09:40:01Z

Pull request #742 was merged, please refactor the test to use the new syntax.

piotro888 · 2024-11-13T18:41:08Z

test/func_blocks/fu/test_lza.py

+                {
+                    "sig_a": help_values.test_val_sig_a_5,
+                    "sig_b": help_values.test_val_sig_b_5,
+                    "carry": 1,
+                },
+                {
+                    "sig_a": help_values.test_val_sig_a_5,
+                    "sig_b": help_values.test_val_sig_b_5,
+                    "carry": 1,
+                },


Two identical test cases.

Maybe it would be better to generate final test cases by adding "carry" to source list of cases? Separate help_values would also not be needed then.

piotro888 · 2024-11-19T15:17:11Z

coreblocks/func_blocks/fu/fpu/lza.py

+def nearestpow2(n):
+    a = int(log2(n))
+    if 2**a == n:
+        return n
+    else:
+        return 2 ** (a + 1)


Amaranth ceil_log2() can be used instead of this function.

Not related to this case as 2**ceil_log2 is short enough to write, but we usually put helper functions that could have use in other places to transactron.utils library

tilk · 2024-12-09T20:47:54Z

test/func_blocks/fu/fpu/test_lza.py

@@ -139,10 +93,16 @@ async def lza_test(sim: TestbenchContext):
                {"shift_amount": 7, "is_zero": 0},
                {"shift_amount": 7, "is_zero": 0},
            ]
-            for i in range(len(test_cases)):
+            for i in range(len(test_cases) // 2):


Why divide by 2? You seem to ignore half of the cases this way. Am I missing something?

(This is one of the reasons why using indexes is discouraged.)

No, it was a mistake on my part. I wanted to remove cases where I explicitly set carry to 1 by performing two tests per test case and manually setting carry to 1 in the second one. For some reason, I decided that dividing the number of test cases by two after I deleted redundant ones is a great idea. But as I said this was a mistake, that is now fixed.

lekcyjna123 · 2024-12-11T09:40:33Z

coreblocks/func_blocks/fu/fpu/lza.py

+
+            m.d.av_comb += t.eq((sig_a ^ sig_b) << 1)
+            m.d.av_comb += g.eq((sig_a & sig_b) << 1)
+            m.d.av_comb += z.eq(((sig_a | sig_b) << 1))


Suggested change

m.d.av_comb += z.eq(((sig_a | sig_b) << 1))

m.d.av_comb += z.eq((sig_a | sig_b) << 1)

tilk marked this pull request as draft October 29, 2024 09:40

Durchbruchswagen marked this pull request as ready for review November 5, 2024 00:08

tilk added the enhancement New feature or request label Nov 5, 2024

tilk added this to the Implement floating point extensions milestone Nov 5, 2024

piotro888 reviewed Nov 10, 2024

View reviewed changes

lekcyjna123 reviewed Nov 10, 2024

View reviewed changes

Durchbruchswagen force-pushed the LZA branch from d46eadd to 4440f6d Compare November 12, 2024 00:06

lekcyjna123 reviewed Nov 16, 2024

View reviewed changes

Durchbruchswagen force-pushed the LZA branch from 51aeafc to 2e0a96b Compare November 18, 2024 22:45

piotro888 reviewed Nov 19, 2024

View reviewed changes

Durchbruchswagen force-pushed the LZA branch from 5355a2e to 4ada0b1 Compare November 25, 2024 20:05

Hazardu mentioned this pull request Nov 26, 2024

Support arbitrary signal length in count_leading_zeros #751

Open

tilk requested review from lekcyjna123 and piotro888 December 3, 2024 09:02

Durchbruchswagen force-pushed the LZA branch from 4ada0b1 to 5788c8f Compare December 9, 2024 18:50

Durchbruchswagen added 10 commits December 9, 2024 20:03

Initial commit

d7566ba

Fixed formatting

86f1b42

Cleaned LZA

eb534f1

Review changes

567a491

Fixed mistakes in LZA description

339d53f

Extension to nearest power of two and 'test_lza' changes

32e003e

Fixed docstring

51ee0d5

Fixed formatting

b3e3e45

Test refactor to new async/await

958395d

Changes to test_lza.py

5788c8f

tilk reviewed Dec 9, 2024

View reviewed changes

Fixed error in tests

e634703

tilk approved these changes Dec 10, 2024

View reviewed changes

lekcyjna123 approved these changes Dec 11, 2024

View reviewed changes

Merge branch 'master' into LZA

517eccc

tilk merged commit 08e43d9 into kuznia-rdzeni:master Dec 14, 2024
13 checks passed

github-actions bot pushed a commit that referenced this pull request Dec 14, 2024

LZA (Leading zeros anticipation) (#741)

f717301

tilk pushed a commit to tilk/coreblocks that referenced this pull request Dec 16, 2024

LZA (Leading zeros anticipation) (kuznia-rdzeni#741)

df115b7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LZA (Leading zeros anticipation) #741

LZA (Leading zeros anticipation) #741

Durchbruchswagen commented Oct 29, 2024 •

edited

Loading

piotro888 Nov 10, 2024

Durchbruchswagen Nov 12, 2024

lekcyjna123 left a comment

lekcyjna123 Nov 10, 2024

Durchbruchswagen Nov 12, 2024

lekcyjna123 Nov 10, 2024

Durchbruchswagen Nov 12, 2024

lekcyjna123 Nov 10, 2024

Durchbruchswagen Nov 12, 2024 •

edited

Loading

lekcyjna123 Nov 10, 2024

Durchbruchswagen Nov 12, 2024

lekcyjna123 Nov 10, 2024

Durchbruchswagen Nov 12, 2024 •

edited

Loading

lekcyjna123 Nov 16, 2024

Durchbruchswagen Nov 18, 2024

lekcyjna123 Nov 10, 2024

Durchbruchswagen Nov 12, 2024

lekcyjna123 Nov 16, 2024

Durchbruchswagen Nov 18, 2024

lekcyjna123 Nov 16, 2024

Durchbruchswagen Nov 18, 2024

tilk commented Nov 19, 2024

piotro888 Nov 13, 2024

Durchbruchswagen Dec 9, 2024

piotro888 Nov 19, 2024 •

edited

Loading

Durchbruchswagen Dec 9, 2024

tilk Dec 9, 2024

Durchbruchswagen Dec 9, 2024

lekcyjna123 Dec 11, 2024

	m.d.av_comb += z.eq(((sig_a \| sig_b) << 1))
	m.d.av_comb += z.eq((sig_a \| sig_b) << 1)

LZA (Leading zeros anticipation) #741

LZA (Leading zeros anticipation) #741

Conversation

Durchbruchswagen commented Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lekcyjna123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Durchbruchswagen Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Durchbruchswagen Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tilk commented Nov 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piotro888 Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Durchbruchswagen commented Oct 29, 2024 •

edited

Loading

Durchbruchswagen Nov 12, 2024 •

edited

Loading

Durchbruchswagen Nov 12, 2024 •

edited

Loading

piotro888 Nov 19, 2024 •

edited

Loading