search

riscvarchive / riscv-binutils-gdb

RISC-V backports for binutils-gdb. Development is done upstream at the FSF.
GNU General Public License v2.0
148 stars 233 forks source link

riscv/sim: Fix order of operations in fnmadd/fnmsub #235

Open bluewww opened 3 years ago

bluewww commented 3 years ago

Previously it was implicitely assumed that float operations are associative. We change the implementation to match exactly the given order in the spec.

Fixes various gcc tests when running those with gdb-sim, see below

                === g++: Unexpected fails for rv32imafc ilp32f medlow ===
FAIL: c-c++-common/torture/complex-sign-mul-minus-one.c   -O2  execution test
FAIL: c-c++-common/torture/complex-sign-mul-minus-one.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
FAIL: c-c++-common/torture/complex-sign-mul-minus-one.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  execution test
FAIL: c-c++-common/torture/complex-sign-mul-minus-one.c   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: c-c++-common/torture/complex-sign-mul-minus-one.c   -O3 -g  execution test
FAIL: c-c++-common/torture/complex-sign-mul-one.c   -O2  execution test
FAIL: c-c++-common/torture/complex-sign-mul-one.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
FAIL: c-c++-common/torture/complex-sign-mul-one.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  execution test
FAIL: c-c++-common/torture/complex-sign-mul-one.c   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: c-c++-common/torture/complex-sign-mul-one.c   -O3 -g  execution test
                === gcc: Unexpected fails for rv32imafc ilp32f medlow ===
FAIL: c-c++-common/torture/complex-sign-mul-minus-one.c   -O2  execution test
FAIL: c-c++-common/torture/complex-sign-mul-minus-one.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
FAIL: c-c++-common/torture/complex-sign-mul-minus-one.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  execution test
FAIL: c-c++-common/torture/complex-sign-mul-minus-one.c   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: c-c++-common/torture/complex-sign-mul-minus-one.c   -O3 -g  execution test
FAIL: c-c++-common/torture/complex-sign-mul-one.c   -O2  execution test
FAIL: c-c++-common/torture/complex-sign-mul-one.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  execution test
FAIL: c-c++-common/torture/complex-sign-mul-one.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  execution test
FAIL: c-c++-common/torture/complex-sign-mul-one.c   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  execution test
FAIL: c-c++-common/torture/complex-sign-mul-one.c   -O3 -g  execution test
jim-wilson commented 3 years ago

Patches for GNU tools require FSF copyright assignments. I can't accept a patch without knowing who you are, and being able to verify that you have a copyright assignment. If you don't have a copyright assignment, and are interested in doing GNU toolchain development, then you can send email assign@gnu.org to get the process started. This may require a signature from your employer if you have one, or your university if you are a student, etc.

Or you can just submit a bug, explain the problem, and let someone with a copyright assignment fix it.

bluewww commented 3 years ago

I will try to get the copyright assignment. GNU emacs accepts small patches withouth such an assignment, but I guess this is not the case here?

jim-wilson commented 3 years ago

Yes, we can accept small patches without an assignment assuming they are pretty obvious, but there is a limit to that, e.g. you can't send multiple small patches to try to get around the copyright assignment requirement. I didn't look at the patch, looking it now, I see it is 8 lines changed which is just below the 10 line limit, and that is 10 lines total, so we probably can't accept another patch from you. I still need a ChangeLog entry, which means I need a name and email address.

Nelson1225 commented 3 years ago

I still need a ChangeLog entry, which means I need a name and email address.

Hi @bluewww, you can refer to other patches, to see the GNU formats of the commit and changlog. It's better to check the commits from FSF binutils or the default branch (riscv-binutils-2.35) in this github. You would probably see some commits from other branches in this github that don't write the detailed descriptions or changlogs. I believe these commits are planed to be sent to FSF mainline someday, and the authors of these commits are all have the copyright assignment, or our companies had finished this for us. Therefore, we will rewrite the missing information for these commit, and then send it to upstream when needed. So it would be great to add the changelog (like @jim-wilson said, we need your name and email) to your commit, once we need to sent it to upstream, then it would be useful, and can also keep your name to credit your original work.

Thanks