[Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Michael Clark]

I need to dig into this. I'll need to take the assertions out, or run with
tracing to see which fcvt test is triggering this unreachable piece of
code. FYI. I can look into it.

$ sh run-riscv-tests.sh
rv64ua-v-amoadd_d
rv64ua-v-amoadd_w
rv64ua-v-amoand_d
rv64ua-v-amoand_w
rv64ua-v-amomax_d
rv64ua-v-amomax_w
rv64ua-v-amomaxu_d
rv64ua-v-amomaxu_w
rv64ua-v-amomin_d
rv64ua-v-amomin_w
rv64ua-v-amominu_d
rv64ua-v-amominu_w
rv64ua-v-amoor_d
rv64ua-v-amoor_w
rv64ua-v-amoswap_d
rv64ua-v-amoswap_w
rv64ua-v-amoxor_d
rv64ua-v-amoxor_w
rv64ua-v-lrsc
rv64uc-v-rvc
rv64ud-v-fadd
rv64ud-v-fclass
rv64ud-v-fcmp
rv64ud-v-fcvt
rv64ud-v-fcvt_w
**
ERROR:/Users/mclark/src/sifive/riscv-qemu/fpu/softfloat.c:1374:round_to_int_and_pack:
code should not be reached
qemu-images/run-tests.sh: line 6: 58437 Abort trap: 6           ${QEMU}
-nographic -machine spike_v1.10 -kernel $i
rv64ud-v-fdiv
rv64ud-v-fmadd
rv64ud-v-fmin
rv64ud-v-ldst
rv64ud-v-move
rv64ud-v-recoding
rv64ud-v-structural
rv64uf-v-fadd
rv64uf-v-fclass
rv64uf-v-fcmp
rv64uf-v-fcvt
rv64uf-v-fcvt_w
**
ERROR:/Users/mclark/src/sifive/riscv-qemu/fpu/softfloat.c:1374:round_to_int_and_pack:
code should not be reached
qemu-images/run-tests.sh: line 6: 58461 Abort trap: 6           ${QEMU}
-nographic -machine spike_v1.10 -kernel $i
rv64uf-v-fdiv
rv64uf-v-fmadd
rv64uf-v-fmin
rv64uf-v-ldst
rv64uf-v-move
rv64uf-v-recoding
rv64ui-v-add
rv64ui-v-addi
rv64ui-v-addiw
rv64ui-v-addw
rv64ui-v-and
rv64ui-v-andi
rv64ui-v-auipc
rv64ui-v-beq
rv64ui-v-bge
rv64ui-v-bgeu
rv64ui-v-blt
rv64ui-v-bltu
rv64ui-v-bne
rv64ui-v-fence_i
rv64ui-v-jal
rv64ui-v-jalr
rv64ui-v-lb
rv64ui-v-lbu
rv64ui-v-ld
rv64ui-v-lh
rv64ui-v-lhu
rv64ui-v-lui
rv64ui-v-lw
rv64ui-v-lwu
rv64ui-v-or
rv64ui-v-ori
rv64ui-v-sb
rv64ui-v-sd
rv64ui-v-sh
rv64ui-v-simple
rv64ui-v-sll
rv64ui-v-slli
rv64ui-v-slliw
rv64ui-v-sllw
rv64ui-v-slt
rv64ui-v-slti
rv64ui-v-sltiu
rv64ui-v-sltu
rv64ui-v-sra
rv64ui-v-srai
rv64ui-v-sraiw
rv64ui-v-sraw
rv64ui-v-srl
rv64ui-v-srli
rv64ui-v-srliw
rv64ui-v-srlw
rv64ui-v-sub
rv64ui-v-subw
rv64ui-v-sw
rv64ui-v-xor
rv64ui-v-xori
rv64um-v-div
rv64um-v-divu
rv64um-v-divuw
rv64um-v-divw
rv64um-v-mul
rv64um-v-mulh
rv64um-v-mulhsu
rv64um-v-mulhu
rv64um-v-mulw
rv64um-v-rem
rv64um-v-remu
rv64um-v-remuw
rv64um-v-remw

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Peter Maydell]

On 9 March 2018 at 04:22, Michael Clark <mjc@sifive.com> wrote:
> I need to dig into this. I'll need to take the assertions out, or run with
> tracing to see which fcvt test is triggering this unreachable piece of
> code. FYI. I can look into it.

> ERROR:/Users/mclark/src/sifive/riscv-qemu/fpu/softfloat.c:1374:round_to_int_and_pack:
> code should not be reached
> qemu-images/run-tests.sh: line 6: 58437 Abort trap: 6           ${QEMU}
> -nographic -machine spike_v1.10 -kernel $i

This looks like maybe it's the issue Richard Jones sent a patch for ?
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg01005.html

thanks
-- PMM

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Michael Clark]

On Fri, 9 Mar 2018 at 11:45 PM, Peter Maydell <peter.maydell@linaro.org>
wrote:

> On 9 March 2018 at 04:22, Michael Clark <mjc@sifive.com> wrote:
> > I need to dig into this. I'll need to take the assertions out, or run
> with
> > tracing to see which fcvt test is triggering this unreachable piece of
> > code. FYI. I can look into it.
>
> >
> ERROR:/Users/mclark/src/sifive/riscv-qemu/fpu/softfloat.c:1374:round_to_int_and_pack:
> > code should not be reached
> > qemu-images/run-tests.sh: line 6: 58437 Abort trap: 6           ${QEMU}
> > -nographic -machine spike_v1.10 -kernel $i
>
> This looks like maybe it's the issue Richard Jones sent a patch for ?
> https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg01005.html


Thanks. Seems I wasn’t on the ‘cc.

We need this patch in our downstream repo... We also need to automate
riscv-tests in the riscv.org repo .travis.yml so we can pick these things
up automatically...

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Philippe Mathieu-Daudé]

Hi Michael,

On 03/09/2018 12:17 PM, Michael Clark wrote:
> On Fri, 9 Mar 2018 at 11:45 PM, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> 
>> On 9 March 2018 at 04:22, Michael Clark <mjc@sifive.com> wrote:
>>> I need to dig into this. I'll need to take the assertions out, or run
>> with
>>> tracing to see which fcvt test is triggering this unreachable piece of
>>> code. FYI. I can look into it.
>>
>>>
>> ERROR:/Users/mclark/src/sifive/riscv-qemu/fpu/softfloat.c:1374:round_to_int_and_pack:
>>> code should not be reached
>>> qemu-images/run-tests.sh: line 6: 58437 Abort trap: 6           ${QEMU}
>>> -nographic -machine spike_v1.10 -kernel $i
>>
>> This looks like maybe it's the issue Richard Jones sent a patch for ?
>> https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg01005.html
> 
> 
> Thanks. Seems I wasn’t on the ‘cc.
> 
> We need this patch in our downstream repo... We also need to automate
> riscv-tests in the riscv.org repo .travis.yml so we can pick these things
> up automatically...

What about upstreaming the QEMU part of your .travis.yml?

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Michael Clark]

On Sat, 10 Mar 2018 at 12:17 AM, Michael Clark <mjc@sifive.com> wrote:

> On Fri, 9 Mar 2018 at 11:45 PM, Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
>> On 9 March 2018 at 04:22, Michael Clark <mjc@sifive.com> wrote:
>> > I need to dig into this. I'll need to take the assertions out, or run
>> with
>> > tracing to see which fcvt test is triggering this unreachable piece of
>> > code. FYI. I can look into it.
>>
>> >
>> ERROR:/Users/mclark/src/sifive/riscv-qemu/fpu/softfloat.c:1374:round_to_int_and_pack:
>> > code should not be reached
>> > qemu-images/run-tests.sh: line 6: 58437 Abort trap: 6           ${QEMU}
>> > -nographic -machine spike_v1.10 -kernel $i
>>
>> This looks like maybe it's the issue Richard Jones sent a patch for ?
>> https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg01005.html
>
>
> Thanks. Seems I wasn’t on the ‘cc.
>
> We need this patch in our downstream repo... We also need to automate
> riscv-tests in the riscv.org repo .travis.yml so we can pick these things
> up automatically...
>

BTW how does one hide signed-off-by or cc email addresses with the
git-send-email workflow?

Seems like editing the patch after git format-patch is likely the only way
around for contributors whose wishes I might not have honoured, except
originally when I was using an advoc albeit buggy workflow, where the Cc’s
et all we’re later added to the patch headers and not the commits. I’ve
probably inadvertently violated someone’s wish to keep their email address
out of the list archives, which is an understandable wish.

This is not an issue with GitHub PR’s as they keep identity information
differently however slicing up the port for upstreaming has lost some of
our contributor history. I have made tags before every squash and rebase so
we can find all history in the riscv repo, as well as previously trying to
keep personal emails out of the cover letters in the first series. With the
change to the git-sendemail  workflow i’ve likely regressed here.

I also probably have to manually edit patches to add ‘Cc to the
riscv-patches mailing list, as it doesn’t seem right to put that email in
the commit messages.

I’ll guess we’ll figure it all out eventually...

Michael

>

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Michael Clark]

On Sat, 10 Mar 2018 at 12:30 AM, Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> Hi Michael,
>
> On 03/09/2018 12:17 PM, Michael Clark wrote:
> > On Fri, 9 Mar 2018 at 11:45 PM, Peter Maydell <peter.maydell@linaro.org>
> > wrote:
> >
> >> On 9 March 2018 at 04:22, Michael Clark <mjc@sifive.com> wrote:
> >>> I need to dig into this. I'll need to take the assertions out, or run
> >> with
> >>> tracing to see which fcvt test is triggering this unreachable piece of
> >>> code. FYI. I can look into it.
> >>
> >>>
> >>
> ERROR:/Users/mclark/src/sifive/riscv-qemu/fpu/softfloat.c:1374:round_to_int_and_pack:
> >>> code should not be reached
> >>> qemu-images/run-tests.sh: line 6: 58437 Abort trap: 6           ${QEMU}
> >>> -nographic -machine spike_v1.10 -kernel $i
> >>
> >> This looks like maybe it's the issue Richard Jones sent a patch for ?
> >> https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg01005.html
> >
> >
> > Thanks. Seems I wasn’t on the ‘cc.
> >
> > We need this patch in our downstream repo... We also need to automate
> > riscv-tests in the riscv.org repo .travis.yml so we can pick these
> things
> > up automatically...
>
> What about upstreaming the QEMU part of your .travis.yml?


We can. It will add overall test coverage as we exercise a lot of the TCG
instrinsics. I’m currently working on dependencies which is having the
riscv-toolchains .travis.yml create toolchain packages in Docker when
tagged using the Travis/GitHub deployment Integration.

The riscv-tests repo can then pull the toolchain into a Docker/repo to
build a binary image of the tools which it can then upload using the same
Travis/GitHub deployment integration.

Unfortunately i’m trying to split my time amongst some CI work on the other
riscv.org repos that i’ve not yet got integrated. My toolchain build recipe
takes 3 hours to make .deb and .rpm multilib toolchain packages on tagging,
which is way beyond the Travis timeouts so i’m exploring some hosted builds
someware, e.g. an AWS instance.

I could just upload binaries manually but we were planning to have this
integrated into GitHub/Travis with tag hooks. We have tested this on some
repositories already, but the toolchain is the main dependency for all of
the other repos to build binary test images that are publicly available.

Currently I have a local set of binaries for testing and it’s adhoc. i.e.
not triggered automatically when we create a tag in one of the repos. We
don’t have anyone dedicated to CI so it’s a task I am working on when time
is available...

Hopefully we’ll have binary images for the various artefacts on the
Releases section on all of the riscv repos, or maybe SiFive will do this in
its own repos. It’s sorely needed as currently everyone has to build
everything from scratch. It takes a lot of time and effort to reproduce a
set of test binaries for riscv-tests, riscv-linux, riscv-pk/bbl and
SiFive’s own E and U series SDKs. The git repos are constantly changing so
the only way at present is to track many repos and rebuild binaries.

It will be great when binaries are included with the GitHub Releases/tags
assuming we use GitHub Releases to host the binaries. I really want to
spend time on this because I think the current CI setup is less than ideal.
I’m frustrated by my own humanity which is by nature to err, which means
sometime forgetting to run exhaustive tests on each release. The obvious
solution is to wire the tests into the CI so bots take care of this.

Michael

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable (-> binary images)

[Liviu Ionescu]

On 9 March 2018 at 13:58:06, Michael Clark (mjc@sifive.com) wrote:

> ... Hopefully we’ll have binary images for the various artefacts
> on the Releases section on all of the riscv repos,
> ...
> It will be great when binaries are included with the GitHub Releases/tags
> assuming we use GitHub Releases to host the binaries.

This is exactly how I did it in GNU MCU Eclipse and it worked great:

- https://github.com/gnu-mcu-eclipse/riscv-none-gcc/releases
- https://github.com/gnu-mcu-eclipse/windows-build-tools/releases
- https://github.com/gnu-mcu-eclipse/openocd/releases
- https://github.com/gnu-mcu-eclipse/qemu/releases

The binaries for QEMU are a bit old, an update is planned in the near future.

And I'll extend the use of xpm with binary xPacks for qemu and
arm-none-eabi-gcc. This will greatly simplify running CI jobs (with
support for Travis and AppVeyor under way).


Regards,

Liviu

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Michael Clark]

BTW I was looking at the IEEE-754 2008 minNum/maxNum code today (the reason
I was running riscv-tests) so that I could forward port the patch to add
IEEE-754 201x minimumNumber/maximumNumber (as there are two pairs of
distinct IEEE-754 functions which have distinct behaviour with respect to
sNaN) and noticed that minNum/maxNum has changed in semantics with respect
to IEEE sNaN for (minNum) but qNaN/sNaN (mininumNumber) semantics aren’t
explicitly mentioned anywhere.

>From my perspective we should still be failing our fmin/fmax test-cases
when calling minNum/maxNum as there is no IEEE-754 201x
minimumNumber/maximumNumber function and there is a specific test for qNaN
vs sNaN. i.e. the new softfloat code may have changed previous default
semantics.

I was working on a patch today but gave up for lack of time. It needs some
further analysis as the softfloat code has changed considerably. It seems
one doesn’t explicitly specify the IEEE-754 function, rather pickNaN which
has replaced propagateFloat in minNum/maxNum (which previously implemented
IEEE-754 minNum/maxNum) and now has per platform specific magic for NaN
behaviour in pickNaN. The original tests for qNaN only in minNum versus
qNaN and sNaN (anyNaN) have been replaced with anyNaN, which was a surprise
to me, as it now reads like IEEE-754 201x minimumNumber/maximumNumber.

Interestingly the RISC-V ISA Test Suite is comprised of tests crafted
against John Hauser’s original softfloat implementation which is used
verbatim by Spike (the RISC-V ISA Simulator). Running the RISC-V Test Suite
in this case is useful for testing IEEE semantics, assuming the tests are
accurate, but it seems the pickNaN approach makes it less clear which IEEE
function a port implements.

The observation is that we are now passing the fmin/fmax test but we are
still calling minNum/maxNum not minimumNumber/maximumNumber.

I suspect FP semenatics might have changed for some other ports so watch
out for floating point conformance issues in the upcoming release.

In any case pickNaN kind of magically works for current set of RISC-V
fmin/fmax tests but I wonder about ports that want IEEE-754 2008. To me,
it’s a case of a test passing that I thought should fail, which is a
warning sign given there is no mention of minNum/maxNum vs
minimumNumber/maximumNumber.

On Sat, 10 Mar 2018 at 12:56 AM, Michael Clark <mjc@sifive.com> wrote:

> On Sat, 10 Mar 2018 at 12:30 AM, Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
>
>> Hi Michael,
>>
>> On 03/09/2018 12:17 PM, Michael Clark wrote:
>> > On Fri, 9 Mar 2018 at 11:45 PM, Peter Maydell <peter.maydell@linaro.org
>> >
>> > wrote:
>> >
>> >> On 9 March 2018 at 04:22, Michael Clark <mjc@sifive.com> wrote:
>> >>> I need to dig into this. I'll need to take the assertions out, or run
>> >> with
>> >>> tracing to see which fcvt test is triggering this unreachable piece of
>> >>> code. FYI. I can look into it.
>> >>
>> >>>
>> >>
>> ERROR:/Users/mclark/src/sifive/riscv-qemu/fpu/softfloat.c:1374:round_to_int_and_pack:
>> >>> code should not be reached
>> >>> qemu-images/run-tests.sh: line 6: 58437 Abort trap: 6
>>  ${QEMU}
>> >>> -nographic -machine spike_v1.10 -kernel $i
>> >>
>> >> This looks like maybe it's the issue Richard Jones sent a patch for ?
>> >> https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg01005.html
>> >
>> >
>> > Thanks. Seems I wasn’t on the ‘cc.
>> >
>> > We need this patch in our downstream repo... We also need to automate
>> > riscv-tests in the riscv.org repo .travis.yml so we can pick these
>> things
>> > up automatically...
>>
>> What about upstreaming the QEMU part of your .travis.yml?
>
>
> We can. It will add overall test coverage as we exercise a lot of the TCG
> instrinsics. I’m currently working on dependencies which is having the
> riscv-toolchains .travis.yml create toolchain packages in Docker when
> tagged using the Travis/GitHub deployment Integration.
>
> The riscv-tests repo can then pull the toolchain into a Docker/repo to
> build a binary image of the tools which it can then upload using the same
> Travis/GitHub deployment integration.
>
> Unfortunately i’m trying to split my time amongst some CI work on the
> other riscv.org repos that i’ve not yet got integrated. My toolchain
> build recipe takes 3 hours to make .deb and .rpm multilib toolchain
> packages on tagging, which is way beyond the Travis timeouts so i’m
> exploring some hosted builds someware, e.g. an AWS instance.
>
> I could just upload binaries manually but we were planning to have this
> integrated into GitHub/Travis with tag hooks. We have tested this on some
> repositories already, but the toolchain is the main dependency for all of
> the other repos to build binary test images that are publicly available.
>
> Currently I have a local set of binaries for testing and it’s adhoc. i.e.
> not triggered automatically when we create a tag in one of the repos. We
> don’t have anyone dedicated to CI so it’s a task I am working on when time
> is available...
>
> Hopefully we’ll have binary images for the various artefacts on the
> Releases section on all of the riscv repos, or maybe SiFive will do this in
> its own repos. It’s sorely needed as currently everyone has to build
> everything from scratch. It takes a lot of time and effort to reproduce a
> set of test binaries for riscv-tests, riscv-linux, riscv-pk/bbl and
> SiFive’s own E and U series SDKs. The git repos are constantly changing so
> the only way at present is to track many repos and rebuild binaries.
>
> It will be great when binaries are included with the GitHub Releases/tags
> assuming we use GitHub Releases to host the binaries. I really want to
> spend time on this because I think the current CI setup is less than ideal.
> I’m frustrated by my own humanity which is by nature to err, which means
> sometime forgetting to run exhaustive tests on each release. The obvious
> solution is to wire the tests into the CI so bots take care of this.
>
> Michael
>

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Emilio G. Cota]

On Fri, Mar 09, 2018 at 11:34:56 +0000, Michael Clark wrote:
> BTW how does one hide signed-off-by or cc email addresses with the
> git-send-email workflow?

You just don't.

> Seems like editing the patch after git format-patch is likely the only way
> around for contributors whose wishes I might not have honoured, except
> originally when I was using an advoc albeit buggy workflow, where the Cc’s
> et all we’re later added to the patch headers and not the commits. I’ve
> probably inadvertently violated someone’s wish to keep their email address
> out of the list archives, which is an understandable wish.

If that's their wish I'd say just use the --cc flag; they get to keep
their address hidden from public commit messages, at the expense of (possibly)
being Cc'ed on more emails than they need to. I think it's a
reasonable trade-off.

That said, if they really want to hide their address from others,
you should use --bcc. But then they wouldn't receive the replies.

> This is not an issue with GitHub PR’s as they keep identity information
> differently however slicing up the port for upstreaming has lost some of
> our contributor history. I have made tags before every squash and rebase so
> we can find all history in the riscv repo, as well as previously trying to
> keep personal emails out of the cover letters in the first series. With the
> change to the git-sendemail  workflow i’ve likely regressed here.
> 
> I also probably have to manually edit patches to add ‘Cc to the
> riscv-patches mailing list, as it doesn’t seem right to put that email in
> the commit messages.

Isn't Cc'ing riscv-patches an obvious use case for using the --cc flag?
(BTW You can add as many --cc's as you want, and these apply to all patches
in a series.)

		E.

PS. As always, remember to use --dry-run =)

Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable

[Palmer Dabbelt]

On Fri, 09 Mar 2018 13:49:57 PST (-0800), cota@braap.org wrote:
> On Fri, Mar 09, 2018 at 11:34:56 +0000, Michael Clark wrote:
> Isn't Cc'ing riscv-patches an obvious use case for using the --cc flag?
> (BTW You can add as many --cc's as you want, and these apply to all patches
> in a series.)

FWIW, that's what I do with my patches.  I generally only add CC lines to the
patches if the address won't be found by the normal means (ie,
scripts/get_maintainer.pl) -- for example, maybe someone helped out a lot with
the patch but hasn't provided a reviewed-by yet.  Everyone else should come out
of the script, which makes it easy to avoid forgetting :).

Of course, that's all for my Linux stuff.  While I haven't really submitted any
QEMU patches, the flow seems very similar to Linux.  The GNU flow is a bit
different, but the git-send-email flow is very similar (without
get_maintainers).