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

use C++ LLVM API rather than C API for array types to support 64 bits #1424

Closed
andrewrk opened this issue Aug 26, 2018 · 6 comments
Closed
Labels
bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals.
Milestone

Comments

@andrewrk
Copy link
Member

See #1422

fn recur() [4294967397]u8 {
    return recur();
}

export fn entry() void {
    var loophole = recur();
}

const builtin = @import("builtin");
pub fn panic(msg: []const u8, error_return_trace: ?*builtin.StackTrace) noreturn {
    while (true) {}
}

the LLVM IR for this is:

; Function Attrs: nobuiltin nounwind
define internal fastcc void @recur([101 x i8]* nonnull sret) unnamed_addr #2 !dbg !53 {
Entry:

The 101 is because it wrapped around 32 bits. This is because the LLVM C API uses unsigned but the C++ API uses uint64_t: static ArrayType *get(Type *ElementType, uint64_t NumElements);

So if we switch to the C++ API to make LLVM array types it fixes this bug.

@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Aug 26, 2018
@andrewrk andrewrk added this to the 0.4.0 milestone Aug 26, 2018
@BarabasGitHub
Copy link
Contributor

I seriously hope nobody will ever need a fixed size array larger than 4GB.

@thejoshwolfe
Copy link
Contributor

I seriously hope nobody will ever need a fixed size array larger than 4GB.

It's definitely possible, especially when you're not running in an OS.

@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Mar 16, 2019
@andrewrk andrewrk added the contributor friendly This issue is limited in scope and/or knowledge of Zig internals. label Mar 16, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Apr 30, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Jan 3, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 10, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 Nov 6, 2020
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 May 19, 2021
@nektro
Copy link
Contributor

nektro commented Jul 12, 2022

triaging upstream as well llvm/llvm-project#56496

@nektro
Copy link
Contributor

nektro commented Feb 15, 2023

I submitted a patch to enable the C api to take advantage of u64 array lengths which has now landed as llvm/llvm-project@35276f1 and will likely make it as part of LLVM 17

@nektro
Copy link
Contributor

nektro commented Sep 19, 2023

completed in 6a07b70

@andrewrk
Copy link
Member Author

Nice work @nektro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals.
Projects
None yet
Development

No branches or pull requests

4 participants