Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xorR is broken in chisel-tester? #78

Closed
zinechant opened this issue Jan 16, 2019 · 5 comments
Closed

xorR is broken in chisel-tester? #78

zinechant opened this issue Jan 16, 2019 · 5 comments
Assignees

Comments

@zinechant
Copy link

Type of issue: bug report

From: chipsalliance/firrtl#993

I am new to chisel world. While I am exploring, I noticed that the xorR is broken in firrtl backend for unit tests.

There were discussions about broken of reduction operators. chipsalliance/firrtl#356
But it seems xorR is still tied to the lowest bit.

A simple repo testing xorR.

Regards,
Chen

@edwardcwang
Copy link

Hello Chen,

Thank you for your interest in Chisel/FIRRTL! Does it work correctly with our newest FIRRTL simulator, treadle? https://github.com/freechipsproject/treadle

@zinechant
Copy link
Author

zinechant commented Jan 16, 2019

Hello

I am using the chisel-template. And if I specify treadle as the backend, xorR is still tied to the lowest bit.

[info] Running xor_reduce.XorReduceMain --backend-name treadle
[info] [0.002] Elaborating design...
[info] [0.096] Done elaborating.
Total FIRRTL Compile Time: 189.9 ms
Total FIRRTL Compile Time: 52.1 ms
file loaded in 0.092280187 seconds, 8 symbols, 2 statements
[info] [0.001] SEED 1547677576725
xor.reduce.io.in1: Vector(0, 0, 0, 0)
xor.reduce.io.out1: 0
xor.reduce.io.in1: Vector(1, 0, 0, 0)
xor.reduce.io.out1: 1
xor.reduce.io.in1: Vector(0, 1, 0, 0)
xor.reduce.io.out1: 0
xor.reduce.io.in1: Vector(1, 1, 0, 0)
xor.reduce.io.out1: 1
xor.reduce.io.in1: Vector(0, 0, 1, 0)
xor.reduce.io.out1: 0
xor.reduce.io.in1: Vector(1, 0, 1, 0)
xor.reduce.io.out1: 1
xor.reduce.io.in1: Vector(0, 1, 1, 0)
xor.reduce.io.out1: 0
xor.reduce.io.in1: Vector(1, 1, 1, 0)
xor.reduce.io.out1: 1
xor.reduce.io.in1: Vector(0, 0, 0, 1)
xor.reduce.io.out1: 0
xor.reduce.io.in1: Vector(1, 0, 0, 1)
xor.reduce.io.out1: 1
xor.reduce.io.in1: Vector(0, 1, 0, 1)
xor.reduce.io.out1: 0
xor.reduce.io.in1: Vector(1, 1, 0, 1)
xor.reduce.io.out1: 1
xor.reduce.io.in1: Vector(0, 0, 1, 1)
xor.reduce.io.out1: 0
xor.reduce.io.in1: Vector(1, 0, 1, 1)
xor.reduce.io.out1: 1
xor.reduce.io.in1: Vector(0, 1, 1, 1)
xor.reduce.io.out1: 0
xor.reduce.io.in1: Vector(1, 1, 1, 1)
xor.reduce.io.out1: 1
test XorReduce Success: 0 tests passed in 5 cycles in 0.018399 seconds 271.76 Hz
[info] [0.008] RAN 0 CYCLES PASSED
[success] Total time: 1 s, completed Jan 16, 2019 4:26:17 PM

I am not sure if this is the correct way to use treadle. But consider that specifying a random backend name would result in an error, I am assuming treadle support is already built into the iotesters.Driver.

Oh, BTW, by using the verilator backend, I get correct xorR behavior.

Regards,
Chen

@chick
Copy link
Collaborator

chick commented Jan 17, 2019

This is definitely a bug, interpreter is using the width of the output to construct and input mask. This is a bad idea.

@chick chick self-assigned this Jan 17, 2019
@chick chick transferred this issue from freechipsproject/firrtl-interpreter Jan 17, 2019
chick added a commit to freechipsproject/firrtl-interpreter that referenced this issue Jan 17, 2019
chick added a commit that referenced this issue Jan 17, 2019
was based on output width of operation instead of input width. This
fixes that problem and adds a test for this error
Fixes Issue #78
@chick
Copy link
Collaborator

chick commented Jan 17, 2019

Thanks for the report, this seems to be a treadle only bug, I have created PR #79 to fix it.

@zinechant
Copy link
Author

zinechant commented Jan 17, 2019

Thanks.
It seems the default backend for iotester.Driver is already treadle.
I confirm that firrtl backend actually delivers the correct behavior of xorR.

chick added a commit that referenced this issue Jan 17, 2019
)

was based on output width of operation instead of input width. This
fixes that problem and adds a test for this error
Fixes Issue #78
@chick chick closed this as completed Feb 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants