Re: [libre-riscv-dev] daily kan-ban update 09jul2020

Re: [libre-riscv-dev] daily kan-ban update 09jul2020

[Luke Kenneth Casson Leighton]

On Fri, Jul 10, 2020 at 10:54 AM Jacob Lifshay <programmerjake@gmail.com> wrote:

> I checked quite a few signals that I know are critical for calculating the
> correct result, such as the ra and rb signals in the *setup* stage, and the
> dividend, divisor_radicand, and operation of DivPipeCore.

so let's track that with the yosys graphvis (show setup).  i can see
ra going in to dividend_neg and abs_dividend (this is visually, not
looking at the code), and i can see rb going in to divisor_neg and
abs_dor (i shortened the name abs_divisor).

however what i *don't* see is ra or rb going into *output* signals
from the setup stage.

now let's look at DivSetupStage (in fu/div/setup_stage.py) and sure enough:

        comb = m.d.comb
        # convenience variables
        op, a, b = self.i.ctx.op, self.i.a, self.i.b
        ...
        ...

        ###### sticky overflow and context, both pass-through #####

        comb += self.o.xer_so.eq(self.i.xer_so)
        comb += self.o.ctx.eq(self.i.ctx)

        return m

at no point is either ra or rb in being copied into ra or rb *out*.
there's no self.o.rb.eq(self.i.rb)

basically what i'm pointing out is that there may not be a bug in nmigen at all.

> true, but they're thrown away after where I checked. I looked through the
> whole list of signals and I think all signals named ra and rb including
> variants like ra$3 were 0 everywhere.

yes, because once the chain is broken (and it looks like it's broken
in DivSetupStage) that's it, zeros will obviously get propagated from
that point onwards in the pipeline.

adding this, the signals are no longer zero, right the way throughout
the entire pipe_N_to_N chain:

diff --git a/src/soc/fu/div/setup_stage.py b/src/soc/fu/div/setup_stage.py
index 25daa201..8975416f 100644
--- a/src/soc/fu/div/setup_stage.py
+++ b/src/soc/fu/div/setup_stage.py
@@ -81,6 +81,9 @@ class DivSetupStage(PipeModBase):

         ###### sticky overflow and context, both pass-through #####

+        comb += self.o.ra.eq(self.i.ra)
+        comb += self.o.rb.eq(self.i.rb)
+
         comb += self.o.xer_so.eq(self.i.xer_so)
         comb += self.o.ctx.eq(self.i.ctx)

we don't actually want to do that because it's 128 extra wires.  those
have to go.  there's currently something like.... 600 wires being
passed between each stage right now.

> > there's nothing wrong with the simulation / pipeline: it's the
> > calculation of overflow.  it's real simple to check: delete these
> > lines
> >
>
> yeah, i got that. I was trying to get a working vcd trace so I could follow
> the overflow computation, which is split between the setup stage and the
> output stage.

i've found it necessary to have *all* the files open on-screen (jobs |
grep vi | wc --> 48 *in one terminal*), have the yosys graphviz open
on a second virtual desktop (full screen, 3840x2160), the vcd trace
open in 1/3 of the screen, *and* do print statements from the
simulation.

the combination - reinforcement of information from multiple sources -
gives the picture in a way that a single tool flat-out cannot.


> If you have time, you could try to build a more minimal reproducer for
> whitequark,

that would take longer than it would to turn the microwatt FSM into
nmigen.  plus, given that the break in the pipeline has been
identified i don't believe it necessary to raise the bugreport.


> since what we have is waay too complicated to be that useful
> for finding the bug in nmigen. if it helps any, I'll put the git revs I am
> on:
> nmigen : 30e2f91176edcd1c8766c2cb11f413b9c77936b9
> ieee754fpu : 610b4a381e70f45f5684cc281398ce77fb5441fa
> nmutil : 3853df675a1e1db24950945f66b076266a7da409
> soc : caceb716e9417ed8731ef08b7b260a4c077186b2
>
> meanwhile, I'll be sleeping for a while.

:)

when you're awake can you make a decision as to whether to investigate
this further or whether to go straight to doing the FSM.

i *strongly recommend* converting divider.vhdl with no actual
"thought".  no cleverness, no "reinvent from scratch".  literally do a
line-for-line conversion of the file.

this because reproduction of all the edge cases - as we're discovering
- is simply too problematic.

microwatt on the other hand has been developed by people with *TWENTY
FIVE YEARS* continuous experience working with the PowerISA in
different forms.  the hardware RTL level was literally the last
remaining area where they didn't have any experience at all.

l.

_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] daily kan-ban update 09jul2020

[Jacob Lifshay]

On Fri, Jul 10, 2020, 05:33 Luke Kenneth Casson Leighton <lkcl@lkcl.net>
wrote:

> On Fri, Jul 10, 2020 at 10:54 AM Jacob Lifshay <programmerjake@gmail.com>
> wrote:
>
> > I checked quite a few signals that I know are critical for calculating
> the
> > correct result, such as the ra and rb signals in the *setup* stage, and
> the
> > dividend, divisor_radicand, and operation of DivPipeCore.
>
> so let's track that with the yosys graphvis (show setup).  i can see
> ra going in to dividend_neg and abs_dividend (this is visually, not
> looking at the code), and i can see rb going in to divisor_neg and
> abs_dor (i shortened the name abs_divisor).
>
> however what i *don't* see is ra or rb going into *output* signals
> from the setup stage.
>

yes, I know and expected that. I'll repeat myself until you get that I was
looking at the ra and rb *inputs* in *this stage* as well as the previous
stage. those *inputs at the beginning of the pipeline* were zero.

at no point is either ra or rb in being copied into ra or rb *out*.
> there's no self.o.rb.eq(self.i.rb)
>
> basically what i'm pointing out is that there may not be a bug in nmigen
> at all.
>
> > true, but they're thrown away after where I checked. I looked through the
> > whole list of signals and I think all signals named ra and rb including
> > variants like ra$3 were 0 everywhere.
>
> yes, because once the chain is broken (and it looks like it's broken
> in DivSetupStage) that's it, zeros will obviously get propagated from
> that point onwards in the pipeline.
>
> adding this, the signals are no longer zero, right the way throughout
> the entire pipe_N_to_N chain:
>

we don't actually want to do that because it's 128 extra wires.  those
> have to go.  there's currently something like.... 600 wires being
> passed between each stage right now.
>

completely agreed.

>
> > > there's nothing wrong with the simulation / pipeline: it's the
> > > calculation of overflow.  it's real simple to check: delete these
> > > lines
> > >
> >
> > yeah, i got that. I was trying to get a working vcd trace so I could
> follow
> > the overflow computation, which is split between the setup stage and the
> > output stage.
>
> i've found it necessary to have *all* the files open on-screen (jobs |
> grep vi | wc --> 48 *in one terminal*), have the yosys graphviz open
> on a second virtual desktop (full screen, 3840x2160), the vcd trace
> open in 1/3 of the screen, *and* do print statements from the
> simulation.
>
> the combination - reinforcement of information from multiple sources -
> gives the picture in a way that a single tool flat-out cannot.
>
>
> > If you have time, you could try to build a more minimal reproducer for
> > whitequark,
>
> that would take longer than it would to turn the microwatt FSM into
> nmigen.



plus, given that the break in the pipeline has been
> identified i don't believe it necessary to raise the bugreport.
>

it *is* necessary to report (unless fixed meanwhile), since you got
distracted by the wrong thing and completely missed that the signals that
shouldn't have been zero since they *are* connected to the pipeline inputs
-- those signals *are* zero in the vcd file.

Jacob
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] daily kan-ban update 09jul2020

[Luke Kenneth Casson Leighton]

On Friday, July 10, 2020, Jacob Lifshay <programmerjake@gmail.com> wrote:

>
> yes, I know and expected that. I'll repeat myself until you get that I was
> looking at the ra and rb *inputs* in *this stage* as well as the previous
> stage. those *inputs at the beginning of the pipeline* were zero.


ok am with you.  that's very odd, because when i added those lines the
correct expected values propagated down the whole thing.

sorry had to narrow down by being able to say what it wasn't :)

there is actually a word for this in vedic epistemology (advaita vedanta)
which is "knowledge by difference".  the discrepancy in a comparison *is*
new knowledge.

took a couple of rounds to get there though :)


>
>
> plus, given that the break in the pipeline has been
> > identified i don't believe it necessary to raise the bugreport.
> >
>
> it *is* necessary to report (unless fixed meanwhile), since you got
> distracted by the wrong thing and completely missed that the signals that
> shouldn't have been zero since they *are* connected to the pipeline inputs
> -- those signals *are* zero in the vcd file.


yes with you now.  sorry took a while.

l.



-- 
---
crowd-funded eco-conscious hardware: https://www.crowdsupply.com/eoma68
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] daily kan-ban update 09jul2020

[Jacob Lifshay]

On Fri, Jul 10, 2020, 10:26 Luke Kenneth Casson Leighton <lkcl@lkcl.net>
wrote:

> On Friday, July 10, 2020, Jacob Lifshay <programmerjake@gmail.com> wrote:
>
> yes with you now.  sorry took a while.
>

opened a bug report:
https://github.com/nmigen/nmigen/issues/429

Jacob
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] daily kan-ban update 09jul2020

[Luke Kenneth Casson Leighton]

On Fri, Jul 10, 2020 at 6:59 PM Jacob Lifshay <programmerjake@gmail.com> wrote:
>
> On Fri, Jul 10, 2020, 10:26 Luke Kenneth Casson Leighton <lkcl@lkcl.net>
> wrote:
>
> > On Friday, July 10, 2020, Jacob Lifshay <programmerjake@gmail.com> wrote:
> >
> > yes with you now.  sorry took a while.
> >
>
> opened a bug report:
> https://github.com/nmigen/nmigen/issues/429

ok let's double-check it.  this pipe_start: (yosys read_ilang
div_simulator.py; show pipe_start):
https://ftp.libre-soc.org/2020-07-10_21-18.png

the chain is therefore (starting from fu/div/pipeline.py):

* DivBasePipe: pipe_start of type DivStagesStart

* DivStagesStart contains
 -        alu_input = DivMulInputStage(self.pspec)
 -      div_setup = DivSetupStage(self.pspec)
 -      core_setup = DivCoreSetupStage(self.pspec)

* DivMulInputStage has a default submodule name of "input"
* DivSetupStage gives its name as "setup_stage"
* DivCoreSetupStage gives its name as "core_setup_stage"

so because DivStagesStart chains those three together, we can confirm
that the input and output parameters - ra, rb, xer_so - are expected
to be copied through in that order.

this from visual inspection of the image, above, is confirmed.  taking ra (only)

* ra starts from pipe_start on the bottom left, gets named input_ra,
goes into input
* comes out of input as input_ra_42, gets named setup_stage_ra, goes
into setup_stage

so we expect

* gtkwave examination of input's ra to be non-zero
* input's ra$1 (which is the *output* from that module) to be non-zeri
* setup_stage's ra to be non-zero
* setup_stage's ra output to... well... exist.  and it doesn't
(because of the missing patch)

ok.  so in this comment:
https://github.com/nmigen/nmigen/issues/429#issuecomment-656814447

you're finding that ra is zero when (and i can confirm here that it is
non-zero) it's expected to be non-zero.

here's where the discrepancy exists: i *am* seeing non-zero values for
top.alu.pipe_start.setup_stage.ra

that's with everything updated on libre-soc side and with this nmigen checkout:
commit 303ea18cb60567e45a755c6b6289a601f27d46e6 (HEAD -> master,
origin/master, origin/HEAD)
Author: Alan Green <alan.green@gmail.com>
Date:   Tue Jun 23 22:12:02 2020 +1000

i've placed a copy of the div_pipeline.il and div_simulator.vcd here:
https://ftp.libre-soc.org/div_pipeline.tgz

l.

_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] daily kan-ban update 09jul2020

[Jacob Lifshay]

On Fri, Jul 10, 2020, 13:44 Luke Kenneth Casson Leighton <lkcl@lkcl.net>
wrote:

> here's where the discrepancy exists: i *am* seeing non-zero values for
> top.alu.pipe_start.setup_stage.ra


> that's with everything updated on libre-soc side and with this nmigen
> checkout:
> commit 303ea18cb60567e45a755c6b6289a601f27d46e6 (HEAD -> master,
> origin/master, origin/HEAD)
> Author: Alan Green <alan.green@gmail.com>
> Date:   Tue Jun 23 22:12:02 2020 +1000
>

thanks! now I have a starting point for bisecting nmigen

Jacob
_______________________________________________
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] bisecting nmigen

[Jacob Lifshay]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

I wrote a helper script to look for the nmigen bug. Attaching it as a
txt file in case anyone wants to look at it, adapt it, whatever.
Didn't put it in a git repo since I'm not sure it should be there.
save the attached script as a .sh file, run chmod +x on it, then, from
the nmigen repo, run:
git bisect start
git bisect bad 30e2f91176edcd1c8766c2cb11f413b9c77936b9
git bisect good 303ea18cb60567e45a755c6b6289a601f27d46e6
git bisect run ~/projects/nmigen-bisect-script.sh &> ../nmigen-bisect.log

Jacob

[-- Attachment #2: nmigen-bisect-script.txt --]
[-- Type: text/plain, Size: 1697 bytes --]

#!/bin/bash
# this script's license is LGPLv2.1+
dir="$(mktemp -d)"
cd "$dir"
git clone ~/projects/nmigen
git clone ~/projects/nmigen-soc
git clone ~/projects/libreriscv/ieee754fpu
git clone ~/projects/libreriscv/nmutil
git clone ~/projects/libreriscv/soc
pushd soc
git submodule init
git config submodule.libreriscv.url ~/projects/libreriscv/soc/libreriscv
git config submodule.nmigen-type-annotations.url ~/projects/libreriscv/soc/nmigen-type-annotations
git submodule update
popd
python3 -m venv "$dir/venv"
(
    set -e
    . "$dir/venv/bin/activate"
    pushd nmigen
    python setup.py develop
    popd
    pushd nmigen-soc
    python setup.py develop
    popd
    pushd nmutil
    python setup.py develop
    popd
    pushd ieee754fpu
    python setup.py develop
    popd
    pushd soc
    make install
    python <<<"from nmigen.back.pysim import Simulator, Delay, Settle"
    popd
)
status=$?
echo "status: $status"
if ((status != 0)); then
    status=125
else
    (
        . "$dir/venv/bin/activate"
        pushd soc
        timeout --kill-after=5s 30s python src/soc/fu/div/test/test_pipe_caller.py DIVTestCase.test_6_regression -f
    )
    if [[ -s soc/div_simulator.vcd ]]; then
        state="$(~/projects/vcdump/target/debug/vcdump --strings soc/div_simulator.vcd |
            jq '.top.alu.pipe_start.setup_stage.ra')"
        echo "state: $state"
        if [[ "$state" == "" || "$state" == "null" ]]; then
            status=125
        elif [[ "$state" =~ ^([^\"]*'"'[^\"]*'"'[^\"]*)*'"''0'*[^0\"][^\"]*'"' ]]; then
            status=0
        else
            status=1
        fi
    else
        status=125
    fi
fi
echo "status: $status"
cd ~
rm -rf "$dir"
exit $status

[-- Attachment #3: Type: text/plain, Size: 177 bytes --]

_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] bisecting nmigen

[Luke Kenneth Casson Leighton]

On Saturday, July 11, 2020, Jacob Lifshay <programmerjake@gmail.com> wrote:

>
> git bisect run ~/projects/nmigen-bisect-script.sh &> ../nmigen-bisect.log


i always wondered how it was intended to be used.  always confused me
though.

plus if there are multiple errors it does not help.

l.



-- 
---
crowd-funded eco-conscious hardware: https://www.crowdsupply.com/eoma68
_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] bisecting nmigen

[Jacob Lifshay]

well, got basically useless results: my script ended up skipping every
single commit except the first and last one, I skipped if it didn't
get far enough to output a vcd file with the signal included.

I'm going to be done for today

Jacob

_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] bisecting nmigen

[Luke Kenneth Casson Leighton]

On Sat, Jul 11, 2020 at 4:04 AM Jacob Lifshay <programmerjake@gmail.com> wrote:
>
> well, got basically useless results: my script ended up skipping every
> single commit except the first and last one, I skipped if it didn't
> get far enough to output a vcd file with the signal included.

sigh this is why i stopped investigating git bisect: the process of
selecting which half of the binary search often needs manual
intervention and inspection, as some bugs are introduced that are not
relevant but otherwise cause a wrong "decision".

l.

_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] bisecting nmigen

[Luke Kenneth Casson Leighton]

commit 2606ee33ad548bed1b9294bbca3962e834d12fd0
Author: whitequark <whitequark@whitequark.org>
Date:   Sun Jun 28 05:04:16 2020 +0000

    back.pysim: simplify.

    Remove _EvalContext, which was a level of indirection serving almost
    no purpose. (The only case where it would be useful is repeatedly

turns out it does serve a purpose: the div simulation hasn't completed
in 10 minutes, where it normally takes... maybe... 25 seconds?  i've
stopped it running.


commit 6d417568ad68565fc7e01e7611ce8cf0b8fdb603
Author: whitequark <whitequark@whitequark.org>
Date:   Wed Jul 8 08:29:20 2020 +0000

    back.pysim: only extract signal names if VCD is requested.

    This commit also fixes an issue introduced in 2606ee33 that regressed
    simulator startup time and bloated VCD files. (It's actually about
    10% faster now than *before* the regression was introduced.)

it's not complete.  i ran a diff on the vcd files: it looks to me like
it's actually dumping zeros into the vcd file.  as in: *actual* zero
values are being outputted.  in other words there's nothing wrong
*with the vcd file itself*: nmigen has *outputted zeros* into a VCD
file that is correctly and legally formatted.

in the meantime whilst that's being sorted, it is necessary to use
this git revision:

commit 303ea18cb60567e45a755c6b6289a601f27d46e6 (HEAD, tag: working2)
Author: Alan Green <alan.green@gmail.com>
Date:   Tue Jun 23 22:12:02 2020 +1000

    _yosys: handle unparseable versions

l.

_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] bisecting nmigen

[Jacob Lifshay]

I found the cause of the bug and submitted a pull request to fix it:
cause: https://github.com/nmigen/nmigen/issues/429#issuecomment-657304913
pull request: https://github.com/nmigen/nmigen/pull/430

Jacob

_______________________________________________
libre-riscv-dev mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev

Re: [libre-riscv-dev] bisecting nmigen

[Luke Kenneth Casson Leighton]

excellent!

On Mon, Jul 13, 2020 at 2:14 AM Jacob Lifshay <programmerjake@gmail.com> wrote:
>
> I found the cause of the bug and submitted a pull request to fix it:
> cause: https://github.com/nmigen/nmigen/issues/429#issuecomment-657304913
> pull request: https://github.com/nmigen/nmigen/pull/430
>
> Jacob
>
> _______________________________________________
> 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 mailing list
libre-riscv-dev@lists.libre-riscv.org
http://lists.libre-riscv.org/mailman/listinfo/libre-riscv-dev