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

Allow multiple nonblocking assignments #321

Closed
chykon opened this issue Mar 20, 2023 · 1 comment · Fixed by #421
Closed

Allow multiple nonblocking assignments #321

chykon opened this issue Mar 20, 2023 · 1 comment · Fixed by #421
Assignees
Labels
enhancement New feature or request

Comments

@chykon
Copy link
Contributor

chykon commented Mar 20, 2023

Motivation

As far as I understood, ROHD is capable of handling multiple consecutive nonblocking assignments to one variable (tried to comment out this line and everything worked). I found mentions of this (#114) and cannot agree with the phrase

It doesn't make sense for a signal to be driven by two drivers in a Sequential block.

Here is an example where I needed this feature:

Sequential(intf.clock, [
  If(~intf.block, then: [
    intf.branch < 0,
    trigger < 0,
    If(trigger, then: [
      IfBlock([
        Iff.s(
          action.eq(CFUInterface.actionAddressLowByte),
          intf.addressLowByte < intf.rfcuData,
        ),
        ElseIf.s(
          action.eq(CFUInterface.actionAddressHighByte),
          intf.addressHighByte < intf.rfcuData,
        ),
        ElseIf.s(
          action.eq(CFUInterface.actionValue),
          value < intf.rfcuData[0],
        ),
        Else.s(
            If(intf.rfcuData[0].eq(CFUInterface.operationSnapshot), then: [
          intf.addressLowByte < instructionAddress.slice(7, 0),
          intf.addressHighByte < instructionAddress.slice(15, 8),
        ], orElse: [
          If.s(value.eq(0), intf.branch < 1)
        ]))
      ]),
    ]),
    If(intf.enable, then: [
      action < intf.action,
      instructionAddress < intf.ifuInstructionAddress,
      trigger < 1,
    ]),
  ]),
]);

In this example, the module saves the control signals and processes them on the next cycle, when the data on the intf.rfcuData bus is ready. The intf.branch and trigger flags are set to zero by default, but this can be overridden depending on conditions. It seems to me that this way of using multiple consecutive non-blocking assignments simplifies the scheme.


Some links:

Desired solution

It is proposed to add functionality for configuring Simulator, for example:

enum SimulatorConfigRestrictionLevel {disallow, notify, allow}

class Simulator {
  // ...

  static void config({SimulatorConfigRestrictionLevel multipleNonblockingAssignments = SimulatorConfigRestrictionLevel.disallow}) {
    // ...
  }

  // ...
}

Prefer default restrictive behavior.

Alternatives considered

No response

Additional details

I can do the implementation if the idea is accepted.

@chykon chykon added the enhancement New feature or request label Mar 20, 2023
@mkorbel1
Copy link
Contributor

Thanks for filing, you make some good points here, and thank you for the reference links. I think I agree with the idea that ROHD should support multiple assignments in Sequential where the last one wins.

I'm not sure a global configuration knob is the right way to handle the behavior change. I'm wondering if the right solution might be:

  • All Sequentials have a default behavior
  • Add an argument in each Sequentials constructor to toggle the behavior.

The value of the "default" for this setting seems like it could go either way. It feels a little "safer" to default to the more restrictive behavior, but if it's unambiguous for simulation and synthesis anyway, why not default to the more flexible behavior? Then if someone wants to restrict themselves, they can enable the additional restriction. Sort of like unique in Case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants