Skip to content

Commit

Permalink
Auto merge of rust-lang#117930 - thomcc:const_str-unnamed, r=nikic
Browse files Browse the repository at this point in the history
Ensure strings created with `const_str` get the `unnamed_addr` attribute

This function (`const_str`) is only used when we need to invent a string during codegen -- for example, for a panic message to pass when codegening some of the assert/panic/etc terminators (for stuff like divide by zero).

AFAICT all other consts, such as the user-defined ones from const eval, should already be getting this attribute (things that come from a ConstAllocation do, for example). Which means that the "unnamed" part is even more true than usual here, these aren't strings that even exist as far as the user can tell.

~~Setting this attribute allows LLVM to merge these constants, leading to significant binary size savings (much more than I would expect). On x86_64-unknown-linux-gnu, t takes a build of ripgrep (release without debug info) from 9.7MiB to 6.0MiB (a savings of over 30%!?), and a build of rustc_driver's shared object from 123MiB to 112MiB (less drastic, but still over 10% reduced).~~

~~The effect on ripgrep is substantially reduced on macOS for reasons beyond me (I may have fucked up the test), only saving around 0.2MiB, although rustc_driver is still around 10MB or smaller than it had been previously.~~

~~This raises some questions, such as "does that mean 1/3 of ripgrep was made of division by zero complaints?" I'm not sure, that may be the case. The output of `strings path/to/rg` is \~2MB smaller, so it seems like a lot of it was. Allowing these to be merged presumably also allow functions that contain them to be merged (if the addresses had semantic meaning, then it stands).~~

~~I intend to do some more analysis here, but I got this up as soon as I realized that this attribute was only missing for internal const strings, and all other ones already get it.~~

Edit: The wins are much more marginal, but there's some argument to do this for the sake of consistency.
  • Loading branch information
bors committed Nov 16, 2023
2 parents 525c91d + 268f5c5 commit 48d8100
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
unsafe {
llvm::LLVMSetInitializer(g, sc);
llvm::LLVMSetGlobalConstant(g, True);
llvm::LLVMSetUnnamedAddress(g, llvm::UnnamedAddr::Global);
llvm::LLVMRustSetLinkage(g, llvm::Linkage::InternalLinkage);
}
(s.to_owned(), g)
Expand Down

0 comments on commit 48d8100

Please sign in to comment.