Skip to content

Add f80 #10639

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

Merged
merged 8 commits into from
Jan 29, 2022
Merged

Add f80 #10639

merged 8 commits into from
Jan 29, 2022

Conversation

Vexu
Copy link
Member

@Vexu Vexu commented Jan 19, 2022

As discussed in Vexu/arocc#213 (comment)

Should calls to compiler-rt on vectors be generated as unrolled or regular loops?

@Vexu Vexu added the stage1 The process of building from source via WebAssembly and the C backend. label Jan 19, 2022
andrewrk
andrewrk previously approved these changes Jan 19, 2022
Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look great 👍

Should calls to compiler-rt on vectors be generated as unrolled or regular loops?

I suggest unrolled, since vectors lengths are generally small numbers. I think the lang spec will allow an implementation to do either way and still be correct.

@Vexu Vexu force-pushed the f80 branch 3 times, most recently from 341606c to 84867ef Compare January 19, 2022 22:36
@andrewrk
Copy link
Member

Some behavior tests to go along with this would be lovely.

@Vexu Vexu force-pushed the f80 branch 4 times, most recently from 3383602 to 9464657 Compare January 20, 2022 17:42
@andrewrk andrewrk force-pushed the f80 branch 2 times, most recently from bc60d8f to e6f7adc Compare January 21, 2022 18:21
@andrewrk andrewrk dismissed their stale review January 21, 2022 19:11

this branch now contains hacks, that need to be removed before merging

@andrewrk
Copy link
Member

Looks like the branch is tripping an LLVM assert. This can be reproduced by building against a debug build of LLVM:

#4  0x000000000801f9e1 in llvm::ConstantExpr::getBitCast (C=0x35a8b170, DstTy=0xeef8268, 
    OnlyIfReduced=false) at /home/andy/Downloads/llvm-project-13/llvm/lib/IR/Constants.cpp:2225
2225      assert(CastInst::castIsValid(Instruction::BitCast, C, DstTy) &&
(gdb) 
#5  0x00000000080467e4 in LLVMConstBitCast (ConstantVal=0x35a8b170, ToType=0xeef8268)
    at /home/andy/Downloads/llvm-project-13/llvm/lib/IR/Core.cpp:1766
1766      return wrap(ConstantExpr::getBitCast(unwrap<Constant>(ConstantVal),
(gdb) 
#6  0x0000000001a4be98 in gen_const_val (g=0xeeeeea0, const_val=0x2c1ee6d0, name=0x2c19e270 "nan_f80")
    at /home/andy/Downloads/zig/src/stage1/codegen.cpp:8100
8100                        return LLVMConstBitCast(as_int, get_llvm_type(g, type_entry));
(gdb) 
#7  0x0000000001a4dede in render_const_val (g=0xeeeeea0, const_val=0x2c1ee6d0, 
    name=0x2c19e270 "nan_f80") at /home/andy/Downloads/zig/src/stage1/codegen.cpp:8560
8560            const_val->llvm_value = gen_const_val(g, const_val, name);
(gdb) 
#8  0x0000000001a4ee03 in do_code_gen (g=0xeeeeea0)
    at /home/andy/Downloads/zig/src/stage1/codegen.cpp:8770
8770                render_const_val(g, var->const_value, symbol_name);
(gdb) p symbol_name
$1 = 0x2c19e270 "nan_f80"
(gdb) 

Vexu and others added 8 commits January 28, 2022 11:45
LLVM bitcast wants integers that match the number of bits. So the const
bitcast has to use an i80, not an i128.

This commit makes the behavior tests fail for me, so it seems I did not
correctly construct the type. But it gets rid of the LLVM segfault.

I noticed that the strategy of memcpy the buf worked if I simply did an
LLVMConstTrunc() on the i128 to make it into an i80 before the
LLVMConstBitCast().

But is that correct in the face of different endianness? I'm not sure.
this way passes the behavior tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants