Behaviour of countLeadingZeros does not match bsg_counting_leading_zeros

[stdavids]

PR #9 replaced the hardfloat module countLeadingZeros with bsg_counting_leading_zeros but the behaviour of these two modules different when the input to the module is 0. For countLeadingZeros, an input of 0 outputs the total number of zeros (ie. the input width). The bsg_counting_leading_zeros module is using a bsg_priority_encode which is defined to the output 0 when the input data is 0.

Have we double checked the logic around the clz replacements to ensure that this mismatch in behavior is never a problem?

[taylor-bsg]

Just a random brainstorm, sometimes you can get this behavior by appending a 1 to the front

…

On Sat, Nov 6, 2021 at 3:09 PM Scott Davidson ***@***.***> wrote: PR #9 <#9> replaced the hardfloat module countLeadingZeros with bsg_counting_leading_zeros but the behaviour of these two modules different when the input to the module is 0. For countLeadingZeros, an input of 0 outputs the total number of zeros (ie. the input width). The bsg_counting_leading_zeros module is using a bsg_priority_encode which is defined to the output 0 when the input data is 0. Have we double checked the logic around the clz replacements to ensure that this mismatch in behavior is never a problem? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#10>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEFG5AD3JLQ3UOX5BU7Y35TUKWYSBANCNFSM5HQBXNGA> .

[stdavids]

Yeah, I had a similar thought and that should mimic the behavior but what is a little strange is that the output wire widths for the clz modules are equal to clog2(in width), not clog2(in width +1). This was true before the PR too, this was not changed. This means when the in width is a power of 2 we are a bit short from being able to count a full input of zeros and so the output is actual 0 like bsg_counting_leading_zeros. This feels like a potential bug in the original code because input width that are a power of 2 and input width that are not a power of 2 are both reasonable. But I'm not very familiar with floating point circuitry so it's hard for me to verify if that is incorrect or if there is logic to make that irrelevant.

Also of note, there are 4 instances of the clz modules, of which PR #9 changed 3. The 3 that were changed have the output width = clog2(in width) but the last one in addRecFN which I was looking at does not have the output width equal to clog2(in width) which is what original sent me down this rabbit hole.

To match the behavior is not too difficult. To be bit accurate we should add a leading 1 to the input and then take the LSBs to match the output wire width if it is smaller or pad zeros if it is larger. But I do wonder if the original code base has a small bug or if this mismatched behavior is irrelevant to the final result.

[taylor-bsg]

The Chisel version of hardfloat uses PriorityEncoder as well. I agree that bsg_count_leading_zeros is incorrect, since it has the wrong output width, and does not count the '0 input correctly. Does addRecFN actually specify the output width corresponding to all of bits set, i.e. one more bit than the other 3?

…

On Sat, Nov 6, 2021 at 11:06 PM Scott Davidson ***@***.***> wrote: Yeah, I had a similar thought and that should mimic the behavior but what is a little strange is that the output wire widths for the clz modules are equal to clog2(in width), not clog2(in width +1). This was true before the PR too, this was not changed. This means when the in width is a power of 2 we are a bit short from being able to count a full input of zeros and so the output is actual 0 like bsg_counting_leading_zeros. This feels like a potential bug in the original code because input width that are a power of 2 and input width that are not a power of 2 are both reasonable. But I'm not very familiar with floating point circuitry so it's hard for me to verify if that is incorrect or if there is logic to make that irrelevant. Also of note, there are 4 instances of the clz modules, of which PR #9 <#9> changed 3. The 3 that were changed have the output width = clog2(in width) but the last one in addRecFN which I was looking at does not have the output width equal to clog2(in width) which is what original sent me down this rabbit hole. To match the behavior is not too difficult. To be bit accurate we should add a leading 1 to the input and then take the LSBs to match the output wire width if it is smaller or pad zeros if it is larger. But I do wonder if the original code base has a small bug or if this mismatched behavior is irrelevant to the final result. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#10 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEFG5ACEXCDOSQKHY4PIOQ3UKYQOBANCNFSM5HQBXNGA> .

[stdavids]

So I double checked all 4 clz in and out widths and put them in this doc https://docs.google.com/spreadsheets/d/1M8CKZMEJJhKqM-jzT4y2IZ_VmmCIHBBKidlU-vbSmYo/edit#gid=0.

What I found was:

1. iNToRecFN and mulAddRecFN have clz out = clog2(clz in)
2. fNToRecFN has clz out = clog2(clz in + 1)
3. addRecFN is neither... but for all legal parameterizations we have clz out >= clog2(clz in + 1)

Edit: corrected equation for 3