-
Notifications
You must be signed in to change notification settings - Fork 23
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
Adds linear feedback shift register component #108
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice start!
@@ -0,0 +1,114 @@ | |||
// Copyright (C) 2023 Intel Corporation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove blank (uncommented) lines in the header?
/// The name of the signal (and output pin) for the [i]th stage. | ||
String _stageName(int i) => '${dataName}_stage_$i'; | ||
|
||
LinearFeedbackShiftRegister(Logic dataIn, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a doc comment to the constructor
|
||
if (reset != null) { | ||
reset = addInput('reset', reset); | ||
if (resetValue != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the original shift register was recently upgraded to allow for a List
resetValue
to control each element of the register (https://github.com/intel/rohd-hcl/blob/main/lib/src/shift_register.dart). However, since the LFSR is always 1-wide, maybe a different API makes sense. The current resetValue
just allows you to basically set all flops to 0 or 1. Should we allow a bitvector of width == shifts
to initialize the LFSR?
Logic lsb = state.getRange(0, 1); // Get LSB (least significant bit) | ||
state = [Const(0, width: 1), state.getRange(1, width)].swizzle(); | ||
|
||
If(lsb.eq(Const(1)), then: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This If
(capitalized) creates a Conditional
object which only executes under certain conditions (e.g. within a Sequential
or Combinational
), but the object is not included in one, so it will never take effect.
state = [Const(0, width: 1), state.getRange(1, width)].swizzle(); | ||
|
||
If(lsb.eq(Const(1)), then: [ | ||
state < state ^ taps // Perform XOR and assign back to state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If taps
is a Logic
, then it should be created as an input to the module. I'm not sure it's necessary to support dynamic re-muxing of the LFSR, in which case the taps could be a non-Logic
(e.g. a List<bool>
, or maybe some int
/BigInt
where the bits correspond to the polynomial
{required Logic clk, | ||
required this.state, | ||
required this.shifts, | ||
required this.taps, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Careful using this.
for inputs of a module, since we want to replace them using addInput
|
||
await lfsr.build(); | ||
|
||
final expectedData = [8, 12, 6, 3, 9, 4, 2, 1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how did you determine the expected values?
Description & Motivation
Added a Galois linear feedback shift register component.
Related Issue(s)
N/A
Testing
Added unit tests for new component. Ran unit tests individually and using dart test.
Backwards-compatibility
No.
Documentation
Will need to add page for LFSR implementation