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

llvm: Use no-builtins attribute instead of nobuiltin. #21897

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

alexrp
Copy link
Member

@alexrp alexrp commented Nov 3, 2024

The former prevents recognizing code patterns and turning them into libcalls, which is what we want for compiler-rt. The latter is meant to be used on call sites to prevent them from being turned into intrinsics.

Context: #21833

@alexrp
Copy link
Member Author

alexrp commented Nov 3, 2024

cc @dweiller

The former prevents recognizing code patterns and turning them into libcalls,
which is what we want for compiler-rt. The latter is meant to be used on call
sites to prevent them from being turned into intrinsics.

Context: ziglang#21833
@dweiller
Copy link
Contributor

dweiller commented Nov 3, 2024

This sounds great - I'll test it out on some code that llvm would turn into stack overflows.

@alexrp
Copy link
Member Author

alexrp commented Nov 4, 2024

Sounds good. Do you feel like doing some testing before I merge?

@dweiller
Copy link
Contributor

dweiller commented Nov 4, 2024

Sounds good. Do you feel like doing some testing before I merge?

I'm doing some testing, but I'm struggling to get llvm to make memcpy recursively call itself even without this patch. It could be that the code patterns that I had doing this no longer trigger this behaviour (I originally saw it happen when zig was on llvm 17 I believe).

@alexrp
Copy link
Member Author

alexrp commented Nov 4, 2024

Maybe try translating one of the Clang examples for this attribute to Zig? https://clang.llvm.org/docs/AttributeReference.html#no-builtin

@alexrp
Copy link
Member Author

alexrp commented Nov 4, 2024

pub fn naive_memset_zero(data: [*]u8, count: usize) callconv(.c) void {
    for (0..count) |i| {
        data[i] = 0;
    }
}

Putting this function in compiler-rt before this PR, its body is replaced with memset(). After this PR, it turns into an auto-vectorized loop instead.

@alexrp alexrp merged commit bd8ef00 into ziglang:master Nov 4, 2024
10 checks passed
@alexrp alexrp deleted the llvm-no-builtins branch November 4, 2024 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants