Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable
On Fri, 09 Mar 2018 13:49:57 PST (-0800), c...@braap.org wrote: On Fri, Mar 09, 2018 at 11:34:56 +, 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).
Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable
On Fri, Mar 09, 2018 at 11:34:56 +, 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
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 wrote: > On Sat, 10 Mar 2018 at 12:30 AM, Philippe Mathieu-Daudé > wrote: > >> Hi Michael, >> >> On 03/09/2018 12:17 PM, Michael Clark wrote: >> > On Fri, 9 Mar 2018 at 11:45 PM, Peter Maydell > > >> > wrote: >> > >> >> On 9 March 2018 at 04:22, Michael Clark 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 artefac
Re: [Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable (-> binary images)
On 9 March 2018 at 13:58:06, Michael Clark (m...@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
On Sat, 10 Mar 2018 at 12:30 AM, Philippe Mathieu-Daudé wrote: > Hi Michael, > > On 03/09/2018 12:17 PM, Michael Clark wrote: > > On Fri, 9 Mar 2018 at 11:45 PM, Peter Maydell > > wrote: > > > >> On 9 March 2018 at 04:22, Michael Clark 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
On Sat, 10 Mar 2018 at 12:17 AM, Michael Clark wrote: > On Fri, 9 Mar 2018 at 11:45 PM, Peter Maydell > wrote: > >> On 9 March 2018 at 04:22, Michael Clark 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
Hi Michael, On 03/09/2018 12:17 PM, Michael Clark wrote: > On Fri, 9 Mar 2018 at 11:45 PM, Peter Maydell > wrote: > >> On 9 March 2018 at 04:22, Michael Clark 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
On Fri, 9 Mar 2018 at 11:45 PM, Peter Maydell wrote: > On 9 March 2018 at 04:22, Michael Clark 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
On 9 March 2018 at 04:22, Michael Clark 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
[Qemu-devel] Apparently fpu/softfloat.c:1374 is reachable
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