Skip to content

Conversation

@compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Feb 18, 2025

Today we emit memcpy/memset llvm intrinsics in rustc_codegen_llvm even when #![no_builtins] is enabled on the crate. This PR adjusts LLVM's memcpy/memset lowering to not do that, since llvm doesn't care to avoid builtins even when the "no-builtins" attribute is enabled on the function.

Curiously, I didn't find an inline version for memmove, lol.

The approach that I took with reading the function attrs on the C++ side is a bit janky, but I couldn't find a good way to thread no_builtins: bool through the various BuilderCx/CodegenCx/Builder/LlvmModule/etc that we have in rustc_codegen_llvm. This seems to be how clang handles lowering inline builtins too -- reading the attrs directly from the function it's building.

I also added support for the "no-builtin-memcpy"/"no-builtin-memmove" attrs even though Rust doesn't emit those today, because it wasn't difficult and it's possible we could add more granularity than #![no_builtins] in the future.

r? @nikic perhaps? I have no idea who else reviews LLVM codegen in rustc, so please reassign if you're busy or not the right person for this. I'm also open to other impl strategies.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 18, 2025
@dianqk
Copy link
Member

dianqk commented Feb 19, 2025

What do you want here? Do you want to avoid calling the memcpy function here?
IIUC, #![no_builtins] is used to prevent LLVM from assuming that functions like memcpy are built-in (or std/known?) functions: https://rust.godbolt.org/z/6YqEqq9n5 and https://clang.godbolt.org/z/sKnWqKqPz.

@nikic
Copy link
Contributor

nikic commented Feb 19, 2025

I agree with @dianqk. The purpose of no_builtin / -fno-builtin is to disable recognition of C library calls. It does not affect whether operations can be legalized using libcalls (keep in mind that this is something that extends far beyond memcpy, it also affects FP math libcalls, and all the good stuff provided by libgcc / compiler-builtins).

@compiler-errors
Copy link
Member Author

Do you want to avoid calling the memcpy function here?

Yep, exactly. Today it's kind of impossible to avoid calling memset even with #[no_builtins]. Is that expected? Do we just accept that users will always need a memset function defined from somewhere? 🤔

@shamatar
Copy link
Contributor

shamatar commented Feb 20, 2025

I tried some no-std code with and without #[no_builtins] and no dependencies:

  • LLVM would still provide memcpy
  • LLVM would still issue calls to it in case of large structures, and recognise manual memcopy-like loops and lower them as generic memcpy call even though alignment and size are known

Also seen very few posts around this topic on forums, and it looks like the main thing wanted is to allow using custom memcpy implementation as a default (even though I didn't dig if it's possible or not, but Godbolt above shows that most likely it is), that may be prevented by this change as The behavior of ‘llvm.memcpy.inline.*’ is equivalent to the behavior of ‘llvm.memcpy.*’, but the generated code is guaranteed not to call any external functions

Another feature wanted would be to be able to have "typed custom memcopy" to again avoid extra alignment and size checks that are present in generic memcopy in case of copying large structures, for which alignment is guaranteed, but it looks impossible

@dianqk
Copy link
Member

dianqk commented Feb 20, 2025

Do you want to avoid calling the memcpy function here?

Yep, exactly. Today it's kind of impossible to avoid calling memset even with #[no_builtins]. Is that expected? Do we just accept that users will always need a memset function defined from somewhere? 🤔

I think this is the expected behavior; someone has to provide the implementation eventually. LLVM assumes that memcpy represents the standard copy logic and attempts to eliminate the call. However, when optimization is failed, the function is still called.

The relationship between memcpy and "no-builtins" is as follows, in LLVM:

  • @memcpy is @llvm.memcpy
  • @memcpy with "no-builtins" is @memcpy
  • @llvm.memcpy with "no-builtins" is @llvm.memcpy

It makes sense to me is that allowing users to use their own provided memcpy everywhere. But I don't know if someone wants that feature. When we do this, LLVMRustBuildMemCpy should use @memcpy instead of @llvm.memcpy.

Also seen very few posts around this topic on forums, and it looks like the main thing wanted is to allow using custom memcpy implementation as a default (even though I didn't dig if it's possible or not, but Godbolt above shows that most likely it is).

Yes. It's possible. Just use @memcpy and "no-builtins" in codegen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants