Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

riscv64 behavior tests stopped passing with LLVM 14 upgrade #12054

Closed
andrewrk opened this issue Jul 9, 2022 · 5 comments · Fixed by #12098
Closed

riscv64 behavior tests stopped passing with LLVM 14 upgrade #12054

andrewrk opened this issue Jul 9, 2022 · 5 comments · Fixed by #12098
Assignees
Labels
arch-riscv 32-bit and 64-bit RISC-V backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness. regression It worked in a previous version of Zig, but stopped working.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Jul 9, 2022

They pass for me locally but stopped working on the CI:

770/1319 test.remainder division... FAIL (TestUnexpectedResult)
771/1319 test.float remainder division using @rem... FAIL (TestUnexpectedResult)
772/1319 test.float modulo division using @mod... FAIL (TestUnexpectedResult)
780/1319 test.@ceil f80... FAIL (TestUnexpectedResult)
781/1319 test.@ceil f128... FAIL (TestUnexpectedResult)
789/1319 test.NaN comparison... FAIL (TestUnexpectedResult)
1223 passed; 90 skipped; 6 failed.
error: the following test command failed with exit code 1:
qemu-riscv64 /workspace/zig-cache/o/ba238760a1396f763444a4cf0e622395/test /workspace/stage2/bin/zig
test...The following command exited with error code 1:
/workspace/stage2/bin/zig test /workspace/test/behavior.zig --test-name-prefix behavior-riscv64-linux-none-Debug-bare-multi-default  --cache-dir /workspace/zig-cache --global-cache-dir /root/.cache/zig --name test -fno-single-threaded -target riscv64-linux-none -mcpu baseline_rv64 --test-cmd qemu-riscv64 --test-cmd-bin -I /workspace/test --zig-lib-dir /workspace/lib --enable-cache 

This issue is to figure out why, fix it, and to re-enable test coverage.

@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior arch-riscv 32-bit and 64-bit RISC-V backend-llvm The LLVM backend outputs an LLVM IR Module. regression It worked in a previous version of Zig, but stopped working. labels Jul 9, 2022
@andrewrk andrewrk added this to the 0.10.0 milestone Jul 9, 2022
@andrewrk andrewrk added the frontend Tokenization, parsing, AstGen, Sema, and Liveness. label Jul 9, 2022
@andrewrk
Copy link
Member Author

andrewrk commented Jul 9, 2022

This affects stage2, not stage1.

@topolarity
Copy link
Contributor

topolarity commented Jul 10, 2022

At least locally, I'm seeing problems on stage 1 as well

My stage 1 problem was resolved by applying https://reviews.llvm.org/D129431 (and also nuking the cache to clear out the old, bad compiler_rt)

I'm now able to reproduce the stage 2-specific issue mentioned here

@topolarity
Copy link
Contributor

Appears to be compiler-rt/C ABI related.

The __cmptf2 generated by stage2 is missing the signext attribute that stage1 gives it (ABI logic added in #8152).

Stepping through with a debugger confirms that the missing sext instruction is one place where the tests diverge on stage 2

@topolarity
Copy link
Contributor

Also, notice that the compiler_rt tests for stage2 are failing in general:

$ ./stage2/bin/zig test lib/compiler_rt.zig -target riscv64-linux-none -mcpu baseline_rv64 --test-cmd qemu-riscv64 --test-cmd-bin --strip
Test [22/235] test.ceil128... FAIL (TestUnexpectedResult)
Test [37/235] test.floor128... FAIL (TestUnexpectedResult)
Test [44/235] test.128... FAIL (TestUnexpectedResult)
Test [64/235] test.round128... FAIL (TestUnexpectedResult)
...
Test [139/235] test.fmodq... FAIL (TestUnexpectedResult)
225 passed; 5 skipped; 5 failed.
error: the following test command failed with exit code 1:
qemu-riscv64 lib/zig-cache/o/bc1b86c1268e5765aaf2c1d2b33b3174/test /home/topolarity/repos/zig/stage2/bin/zig

@andrewrk andrewrk mentioned this issue Jul 13, 2022
5 tasks
@andrewrk andrewrk self-assigned this Jul 13, 2022
@andrewrk
Copy link
Member Author

Appears to be compiler-rt/C ABI related.

The __cmptf2 generated by stage2 is missing the signext attribute that stage1 gives it (ABI logic added in #8152).

Stepping through with a debugger confirms that the missing sext instruction is one place where the tests diverge on stage 2

This comment was spot on! Fix coming right up...

andrewrk added a commit that referenced this issue Jul 13, 2022
For calling convention ABI purposes, integer attributes and return
values need to have an LLVM attribute signext or zeroext added
sometimes. This commit implements that logic.

It also implements a proof-of-concept of moving the F16T type from
being a compiler_rt hack to being how the compiler lowers f16 in
functions that need to match certain calling conventions.

Closes #12054
andrewrk added a commit that referenced this issue Jul 13, 2022
For calling convention ABI purposes, integer attributes and return
values need to have an LLVM attribute signext or zeroext added
sometimes. This commit implements that logic.

It also implements a proof-of-concept of moving the F16T type from
being a compiler_rt hack to being how the compiler lowers f16 in
functions that need to match certain calling conventions.

Closes #12054
andrewrk added a commit that referenced this issue Jul 19, 2022
For calling convention ABI purposes, integer attributes and return
values need to have an LLVM attribute signext or zeroext added
sometimes. This commit implements that logic.

It also implements a proof-of-concept of moving the F16T type from
being a compiler_rt hack to being how the compiler lowers f16 in
functions that need to match certain calling conventions.

Closes #12054
wooster0 pushed a commit to wooster0/zig that referenced this issue Jul 24, 2022
wooster0 pushed a commit to wooster0/zig that referenced this issue Jul 24, 2022
For calling convention ABI purposes, integer attributes and return
values need to have an LLVM attribute signext or zeroext added
sometimes. This commit implements that logic.

It also implements a proof-of-concept of moving the F16T type from
being a compiler_rt hack to being how the compiler lowers f16 in
functions that need to match certain calling conventions.

Closes ziglang#12054
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-riscv 32-bit and 64-bit RISC-V backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness. regression It worked in a previous version of Zig, but stopped working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants