[libre-riscv-dev] [Bug 271] New: SigDecode in power_fields has extra spurious fields

[libre-riscv-dev] [Bug 271] New: SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

            Bug ID: 271
           Summary: SigDecode in power_fields has extra spurious fields
           Product: Libre-SOC's first SoC
           Version: unspecified
          Hardware: PC
                OS: Linux
            Status: CONFIRMED
          Severity: enhancement
          Priority: ---
         Component: Source Code
          Assignee: lkcl@lkcl.net
          Reporter: lkcl@lkcl.net
                CC: libre-riscv-dev@lists.libre-riscv.org
   NLnet milestone: ---

hi michael,

SigDecode has had these added to it:

        comb += self.x_s.eq(self.df.FormX.S[0])
        comb += self.x_sh.eq(self.df.FormX.SH[0:-1])
        comb += self.dq_xs_s.eq(self.df.FormDQ.SX_S[0:-1])

DecodeFields itself is where i was intending to be the canonical location
for adding these kinds of things.  if you feel that they should be
added as signals, then we should do them in an automated fashion,
because otherwise, after a hundred instructions are added, SigDecode
will be jammed with almost 200 signal fields... *all of which were added
by hand*.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

Michael Nolan <mtnolan2640@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|CONFIRMED                   |RESOLVED

--- Comment #1 from Michael Nolan <mtnolan2640@gmail.com> ---
Fixed in 1c9a58b

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #2 from Luke Kenneth Casson Leighton <lkcl@lkcl.net> ---
https://git.libre-riscv.org/?p=soc.git;a=summary

remember ti git push it tho :)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #3 from Luke Kenneth Casson Leighton <lkcl@lkcl.net> ---
what do you think of the idea of adding signals with the same name as
the fields?

btw i'm starting that mini-parser and will be looking to drop a dictionary
of the name of the instruction's form (Form-X) into the exec context, to
test them.

so actually creating signals (again, automatically and/or programmatically)
makes things slightly easier.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #4 from Michael Nolan <mtnolan2640@gmail.com> ---
(In reply to Luke Kenneth Casson Leighton from comment #3)
> what do you think of the idea of adding signals with the same name as
> the fields?


Haven't you already done this with power_fields/power_fieldsn?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #5 from Luke Kenneth Casson Leighton <lkcl@lkcl.net> ---
(In reply to Michael Nolan from comment #4)
> (In reply to Luke Kenneth Casson Leighton from comment #3)
> > what do you think of the idea of adding signals with the same name as
> > the fields?
> 
> 
> Haven't you already done this with power_fields/power_fieldsn?

if you recall i mentioned this a month ago particularly when we did those
recursive expressions.  the distinction between nmigen expressions and nmigen
signals is very important.

they are expressions rather than signals
which, if you recall, results in duplication of those expressions wherever they
are used (hence the massivr graphs ladt month despite the code only being a few
lines long)

if the expressions are used a hundred times then they are copied into the HDL a
hindred times because they are a nmigen AST tree fragment.

as signals, the *signal* would be copied and would show up in the yosys graphs.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #6 from Michael Nolan <mtnolan2640@gmail.com> ---
(In reply to Luke Kenneth Casson Leighton from comment #5)
> they are expressions rather than signals
> which, if you recall, results in duplication of those expressions wherever
> they are used (hence the massivr graphs ladt month despite the code only
> being a few lines long)
> 
Ah, I see now

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #7 from Michael Nolan <mtnolan2640@gmail.com> ---
Ok, I've got it sort of working in df295b5. I removed TopPowerDecoder's
inheritance of DecodeFields and moved DecodeFields to a member. I then had
TopPowerDecoder go through each of the "common" fields in DecodeFields and copy
it to a signal. This works, but it doesn't take care of the form specific
fields (e.g. dec.FormXL.XO[9]), and I'm not sure what to do with those. I
suppose I could add in submodules for each of the forms and copy their signals
like I did with the common ones, would that work for you?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #8 from Luke Kenneth Casson Leighton <lkcl@lkcl.net> ---
(In reply to Michael Nolan from comment #7)
> Ok, I've got it sort of working in df295b5. I removed TopPowerDecoder's
> inheritance of DecodeFields and moved DecodeFields to a member. I then had
> TopPowerDecoder go through each of the "common" fields in DecodeFields and
> copy it to a signal. 

great.

> This works, but it doesn't take care of the form
> specific fields (e.g. dec.FormXL.XO[9]), and I'm not sure what to do with
> those. I suppose I could add in submodules for each of the forms and copy
> their signals like I did with the common ones, would that work for you?

submodules is unnecessary (and a bit overkill): an object that is a dummy class
would do (similar to the use of NamedTuple)

class Form:
    pass

or, actually, just a NamedTuple again.  hmm take a look see what you
think

commit 5374cb82b

_should_ be accessible as

comb += something.eq(decoder.FormX.RA)

which becomes important in the autogenerator code because each instruction
is listed as having a "Form", and we can drop the decoder.Form{something}
into its context, and the pseudo-code fragment will go "oh, i recognise
these variables RA, RT etc. as being from Form{something}"

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #9 from Michael Nolan <mtnolan2640@gmail.com> ---
Interesting, it looks like we're going to need to remove the [0:-1] from
anything that uses the fields now.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #10 from Michael Nolan <mtnolan2640@gmail.com> ---
(In reply to Michael Nolan from comment #9)
> Interesting, it looks like we're going to need to remove the [0:-1] from
> anything that uses the fields now.

Oh right, that removes the last value from the signal normally. Anyways, all
the tests pass now

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #11 from Luke Kenneth Casson Leighton <lkcl@lkcl.net> ---
(In reply to Michael Nolan from comment #9)
> Interesting, it looks like we're going to need to remove the [0:-1] from
> anything that uses the fields now.

it was a bad hack, i couldn't work out how to just refer to the field
without going through __getattr__.

now that everything goes *through* the equivalent of that, and is now
sub-signals (slices of the original opcode), yes that hack should not
be necessary... *as long* as we refer to those fields.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

[libre-riscv-dev] [Bug 271] SigDecode in power_fields has extra spurious fields

[bugzilla-daemon]

http://bugs.libre-riscv.org/show_bug.cgi?id=271

--- Comment #12 from Luke Kenneth Casson Leighton <lkcl@lkcl.net> ---
(In reply to Luke Kenneth Casson Leighton from comment #11)

> be necessary... *as long* as we refer to those fields.

correction: sub-signals.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev