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

document coverUnreachable and strict options for the switch block #139

Merged
merged 11 commits into from
Nov 29, 2022
60 changes: 60 additions & 0 deletions source/SpinalHDL/Semantic/when_switch.rst
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,66 @@ is equivalent to
}
}


Additional options
^^^^^^^^^^^^^^^^^^

In some cases when covering all cases of the input encoding still leaves uncovered codes.
In the generated HDL these will be handled by default with the last ``is`` block.
saahm marked this conversation as resolved.
Show resolved Hide resolved
To explicitly declare and define a ``default`` block the option ``coverUnreachable`` can be passed to the switch
Copy link
Collaborator

@numero-744 numero-744 Nov 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure to understand correctly what it is about here so I suggest another way to explain.

If I understand correctly there are two kinds of "cover" to do (I use my words here but feel free to use other words if they are clearer):

  • "logic" cover: the possible values that can be applied on the wires from the logic (enumerated types)
  • "bits" cover: all possible combination of 0s and 1s in the wires. It is a superset of "logic" cover.

For example, using this encoding in an enumerated type (maybe provide an example with actual code)

MOV -> b00
ADD -> b10
SUB -> b11

"logic" cover requires covering the 3 cases MOV, ADD and SUB and "bits" cover requires covering the 4 cases b00 to b11, which are the 3 cases from "logic" cover + unreachable case b01.

By default SpinalHDL only checks "logic" coverage. To reach full "bits" coverage, unreachable cases are redirected to the last case of the switch (note: it can be an is as well as a default?).

If you have defined is cases with a complete "logic" cover and want to define a specific behavior for unreachable cases, you will add a default case; but as all reachable cases were already covered, it will be considered unreachable. To not consider it unreachable and use it for unreachable cases, use coverUnreachable = true.

Example…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so it matters if I use an enumerate (logic cover) or just bits (bits cover) [regardless of constants or immediate values].
I will try to rewrite it then to make it a bit more close to that understanding. Maybe I can also come up with a good example to code show this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it is just my understanding and I do not use it so maybe my understanding is wrong.

@Dolu1990

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by default, will handle the uncovered cases with the last is block.

That sound wrong or else confusing to me.

The goal here is to document coverUnreachable right ?

What's about something kind of like :

"""
If a switch/is contains a 'default' statement while already all the possible values of the 'switch' are covered by the 'is' values, SpinalHDL will generate an "UNREACHABLE DEFAULT STATEMENT" error. You can drop this error reporting by specifying switch(myValue, coverUnreachable = true){ ... }.
"""

?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dolu1990 was my suggestion correct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say, this should not be documented, as it is mostly a generation artefact, not realy a feature.

I 80% agree so I agree XD

Also, notes SpinalHDL replace the last "is" with a "default" only if all the logical values are covered by the "is" statements

Okay now I 100% agree to just don't tell about it then XD XD

So for the explanation of coverUnreachable, I suggest:

  1. Explain logic vs bits coverage
  2. Mention that to manage bits coverage a user-defined way (do not mention what Spinal does by default) if all logic coverage is done with only is statements, have to use coverUnreachable with a default statement.

Are you okay with it @Dolu1990 and @saahm ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will adapt it then according to what I see here. I couldnt follow this in the past few days but I will catch up and propose a respective change :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@numero-744 Seems good to me ^^

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So i had time now to read a bit here, what I conclude is the documentation wont talk about all of the details we discussed about here but what @numero-744 concluded "show and explain logic(enum?) vs bits" and the small thing about coverage of all logic cases by using coverUnreachable for some corner cases of switch-is


.. code-block:: scala

switch(aluop, coverUnreachable = true) {
is(ALUOp.add, ALUOp.slt, ALUOp.sltu) {
immediate := instruction.immI.signExtend
}
is(ALUOp.sll, ALUOp.sra) {
immediate := instruction.shamt
}
default{
immediate := 0
}
}

If the used values of ``ALUOp`` are all available elements of the SpinalEnum type, then without the ``coverUnreachable = true`` option SpinalHDL would throw an UNREACHABLE STATEMENT error upon elaboration.
Without the ``default`` block the encoding for ``ALUOp.sll`` and ``ALUOp.sra`` are the default cases in the generated HDL.

When using defined constants to compare against within a ``switch`` block it can occasionally happen that duplicates within a ``is`` value occur.
By default duplicates of ``is`` conditions are not ignored but identified as an error.
To relax the strictness of the ``switch`` elaboration the ``strict = false`` can be passed (by default ``strict = true`` thus preventing duplicate ``is`` conditions).

.. code-block:: scala
numero-744 marked this conversation as resolved.
Show resolved Hide resolved

// OP_ADD and OP_SUB share the same code
def OP_ADD = M"000"
def OP_SUB = M"000"
def OP_SLT = M"001"
def OP_JMP = M"010"
def OP_BRK = M"101"
val foo = UInt(8 bits)
val bar = UInt(8 bits)
switch(io.instruction, strict=false){
saahm marked this conversation as resolved.
Show resolved Hide resolved
//
is(OP_ADD, OP_SUB){
saahm marked this conversation as resolved.
Show resolved Hide resolved
foo := 4
bar := 2
}
is(OP_SLT){
saahm marked this conversation as resolved.
Show resolved Hide resolved
foo := 2
bar := 8
}
is(OP_JMP, OP_BRK){
saahm marked this conversation as resolved.
Show resolved Hide resolved
foo := 2
bar := 8
}
default{
saahm marked this conversation as resolved.
Show resolved Hide resolved
foo := 0
bar := 0
}
}


Local declaration
-----------------

Expand Down