Skip to content
This repository has been archived by the owner on Aug 17, 2022. It is now read-only.

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

Open
wants to merge 1 commit into
base: fsf-gdb-9.1-with-sim
Choose a base branch
from
Open

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

wants to merge 1 commit into from

Conversation

bluewww
Copy link

@bluewww bluewww commented Oct 29, 2020

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

Previously it was implicitely assumed that float operations are
associative. We change the implementation to match exactly the given
order in the spec.
@jim-wilson
Copy link
Collaborator

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
Copy link
Author

bluewww commented Oct 29, 2020

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
Copy link
Collaborator

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
Copy link
Collaborator

Nelson1225 commented Nov 2, 2020

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants