Don't use `llvm_type` as the type of `static`s. by beetrees · Pull Request #165371 · rust-lang/rust

beetrees · 2026-11-02T19:00:54Z

This PR replaces the two uses of llvm_type as the type of statics - only the size of the static matters, so cx.type_array(cx.type_i8(), layout.size) is sufficient. As statics have to be redefined if the initialiser type is not the same as the existing type. using an LLVM i8 array here is more optimal as it means the static type already matches the initializer in cases where the initializer is just plain bytes.

r? @folkertdev

bjorn3 · 2026-11-02T20:17:08Z

Wouldn't this just cause the logic at

rust/compiler/rustc_codegen_llvm/src/consts.rs

Lines 497 to 528 in ae36fe2

    
           // codegen_static_initializer creates the global value just from the 
        
           // `Allocation` data by generating one big struct value that is just 
        
           // all the bytes and pointers after each other. This will almost never 
        
           // match the type that the static was declared with. Unfortunately 
        
           // we can't just LLVMConstBitCast our way out of it because that has very 
        
           // specific rules on what can be cast. So instead of adding a new way to 
        
           // generate static initializers that match the static's type, we picked 
        
           // the easier option and retroactively change the type of the static item itself. 
        
           let name = String::from_utf8(llvm::get_value_name(g)) 
        
               .expect("we declare our statics with a utf8-valid name"); 
        
           llvm::set_value_name(g, b""); 
        
           let linkage = llvm::get_linkage(g); 
        
           let visibility = llvm::get_visibility(g); 
        
           let new_g = self.declare_global(&name, val_llty); 
        
           llvm::set_linkage(new_g, linkage); 
        
           llvm::set_visibility(new_g, visibility); 
        
           // The old global has had its name removed but is returned by 
        
           // get_static since it is in the instance cache. Provide an 
        
           // alternative lookup that points to the new global so that 
        
           // global_asm! can compute the correct mangled symbol name 
        
           // for the global. 
        
           self.renamed_statics.borrow_mut().insert(def_id, new_g); 
        
           // To avoid breaking any invariants, we leave around the old 
        
           // global for the moment; we'll replace all references to it 
        
           // with the new global later. (See base::codegen_backend.) 
        
           self.statics_to_rauw.borrow_mut().push((g, new_g)); 
        
           new_g

to run unconditionally rather than only for the couple of cases where the initializer happened to have a different shape than the type suggests?

beetrees · 2026-11-02T20:24:59Z

This PR causes that logic to run less, not more. Initializers don't have the type of the variable - they're just an of array of bytes interspersed with pointers and arrays of undef. In the case of an initializer with no undef or pointers, the initializer type will be [size x i8], which happens to match the type I chose here.

folkertdev

One question, if you agree it's not a problem then r=me

This runs the risk of tripping up all sorts of LLVM tests, this one seems the most likely to me

@bors try jobs=test-x86_64-gnu-nopt

View changes since this review

folkertdev · 2026-11-02T21:04:42Z

terrible name...

I've renamed them to actual_global and pointer_global respectively.

folkertdev · 2026-11-02T21:19:56Z

+// A64-NEXT: ldr     {{[a-z0-9]+}}, {{\[}}[[REG2]]{{\]}}
 #[no_mangle]
 pub fn mango() -> u8 {
    unsafe { *EXOCHORDA }


curiously, mangoes are not in exochorda.

Anyway, are we worried about this additional instruction? The docs note that the address given to ldr

Is the optional positive immediate byte offset, a multiple of 8 in the range 0 to 32760, defaulting to 0.

So I guess the address of EXOCHORDA is not longer appropriately aligned? Which, I suppose it was by luck before given that u8 has an alignment of 1 anyway?

Huh, at least it's pretty.

The ldr is the load of the *mut u8 itself (which is the type of the static), the u8 is loaded by a ldrb afterwards. I'm guessing LLVM is presuming the static has an alignment based on the default alignment of the type of the static - this seems incorrect as BackendRepr::Scalar doesn't provide any alignment guarantees. This PR incidentally fixes this by always using a type with an alignment of 1 - I think not fixing this would mean that miscompilations would occur after #159033 is merged.

The LangRef says

An explicit alignment may be specified for a global, which must be a power of 2. If not present, or if the alignment is set to zero, the alignment of the global is set by the target to whatever it feels convenient.

which isn't especially reassuring. This PR is an improvement upon the status quo, but is there any reason we don't set an explicit alignment on global declarations?

There is no reason as far as I am aware of, and I know most all of the other curses in this area. Globals should receive explicitly-specified alignments for the same reason everything else gets explicitly-specified alignments.

Don't use `llvm_type` as the type of `static`s. try-job: test-x86_64-gnu-nopt

beetrees · 2026-11-02T22:25:47Z

Per #165371 (review)

@bors r=folkertdev

rust-bors · 2026-11-02T22:25:50Z

📌 Commit aff3637 has been tentatively approved by folkertdev

It will be put into the queue for this repository once PR CI succeeds.

rust-bors · 2026-11-02T23:01:21Z

☀️ Try build successful (CI)
Build commit: 6395ce3 (6395ce3db55267550ec8641429c257264d52e598)
Base parent: 1c3a650 (1c3a6506966e21845b31de0b075d2527660db132)

@folkertdev

…rtdev Don't use `llvm_type` as the type of `static`s. This PR replaces the two uses of `llvm_type` as the type of `static`s - only the size of the static matters, so `cx.type_array(cx.type_i8(), layout.size)` is sufficient. As `static`s have to be redefined if the initialiser type is not the same as the existing type. using an LLVM `i8` array here is more optimal as it means the static type already matches the initializer in cases where the initializer is just plain bytes. r? @folkertdev

@folkertdev

…rtdev Don't use `llvm_type` as the type of `static`s. This PR replaces the two uses of `llvm_type` as the type of `static`s - only the size of the static matters, so `cx.type_array(cx.type_i8(), layout.size)` is sufficient. As `static`s have to be redefined if the initialiser type is not the same as the existing type. using an LLVM `i8` array here is more optimal as it means the static type already matches the initializer in cases where the initializer is just plain bytes. r? @folkertdev

…uwer Rollup of 15 pull requests Successful merges: - #165378 (rustfmt subtree update (v1.12.0)) - #149245 (Make inline attributes apply to the generated `poll` in async fn) - #164304 ([perf] Reuse CanonicalVarKinds if no new variables were added) - #165109 (Avoid building a region map when erasing bound regions) - #163393 (std: Use getexecpath() for current_exe() on OpenBSD) - #164896 (offload: pass the linked device image to the host pass) - #165358 (Reenable some cg_gcc tests that were disabled) - #165371 (Don't use `llvm_type` as the type of `static`s.) - #162232 (Replace `builtin #` with `k#`) - #162608 (Avoid oscillating line wrapping due to symbol indices) - #165029 (Use `MalformedLintAttributeInput` in more places) - #165203 (AIX: Implement TLS destructor guard in std and update dynamic-loading-cdylib test) - #165230 (Fix rigidity assertion ICE) - #165354 (Remove `AbiAlign`) - #165374 (Refactorings in preparation for the move trait)

@folkertdev

…rtdev Don't use `llvm_type` as the type of `static`s. This PR replaces the two uses of `llvm_type` as the type of `static`s - only the size of the static matters, so `cx.type_array(cx.type_i8(), layout.size)` is sufficient. As `static`s have to be redefined if the initialiser type is not the same as the existing type. using an LLVM `i8` array here is more optimal as it means the static type already matches the initializer in cases where the initializer is just plain bytes. r? @folkertdev

…uwer Rollup of 14 pull requests Successful merges: - #165378 (rustfmt subtree update (v1.12.0)) - #149245 (Make inline attributes apply to the generated `poll` in async fn) - #164304 ([perf] Reuse CanonicalVarKinds if no new variables were added) - #165109 (Avoid building a region map when erasing bound regions) - #163393 (std: Use getexecpath() for current_exe() on OpenBSD) - #164896 (offload: pass the linked device image to the host pass) - #165358 (Reenable some cg_gcc tests that were disabled) - #165371 (Don't use `llvm_type` as the type of `static`s.) - #162232 (Replace `builtin #` with `k#`) - #162608 (Avoid oscillating line wrapping due to symbol indices) - #165029 (Use `MalformedLintAttributeInput` in more places) - #165203 (AIX: Implement TLS destructor guard in std and update dynamic-loading-cdylib test) - #165230 (Fix rigidity assertion ICE) - #165374 (Refactorings in preparation for the move trait)

@folkertdev

…rtdev Don't use `llvm_type` as the type of `static`s. This PR replaces the two uses of `llvm_type` as the type of `static`s - only the size of the static matters, so `cx.type_array(cx.type_i8(), layout.size)` is sufficient. As `static`s have to be redefined if the initialiser type is not the same as the existing type. using an LLVM `i8` array here is more optimal as it means the static type already matches the initializer in cases where the initializer is just plain bytes. r? @folkertdev

…uwer Rollup of 14 pull requests Successful merges: - #149245 (Make inline attributes apply to the generated `poll` in async fn) - #164304 ([perf] Reuse CanonicalVarKinds if no new variables were added) - #165109 (Avoid building a region map when erasing bound regions) - #163393 (std: Use getexecpath() for current_exe() on OpenBSD) - #164896 (offload: pass the linked device image to the host pass) - #165358 (Reenable some cg_gcc tests that were disabled) - #165371 (Don't use `llvm_type` as the type of `static`s.) - #162232 (Replace `builtin #` with `k#`) - #162608 (Avoid oscillating line wrapping due to symbol indices) - #164241 (make the safety documentation consistent in Ptr module) - #165029 (Use `MalformedLintAttributeInput` in more places) - #165203 (AIX: Implement TLS destructor guard in std and update dynamic-loading-cdylib test) - #165230 (Fix rigidity assertion ICE) - #165374 (Refactorings in preparation for the move trait)

@folkertdev

…rtdev Don't use `llvm_type` as the type of `static`s. This PR replaces the two uses of `llvm_type` as the type of `static`s - only the size of the static matters, so `cx.type_array(cx.type_i8(), layout.size)` is sufficient. As `static`s have to be redefined if the initialiser type is not the same as the existing type. using an LLVM `i8` array here is more optimal as it means the static type already matches the initializer in cases where the initializer is just plain bytes. r? @folkertdev

…uwer Rollup of 13 pull requests Successful merges: - #149245 (Make inline attributes apply to the generated `poll` in async fn) - #164304 ([perf] Reuse CanonicalVarKinds if no new variables were added) - #165109 (Avoid building a region map when erasing bound regions) - #163393 (std: Use getexecpath() for current_exe() on OpenBSD) - #165358 (Reenable some cg_gcc tests that were disabled) - #165371 (Don't use `llvm_type` as the type of `static`s.) - #162232 (Replace `builtin #` with `k#`) - #162608 (Avoid oscillating line wrapping due to symbol indices) - #164241 (make the safety documentation consistent in Ptr module) - #165029 (Use `MalformedLintAttributeInput` in more places) - #165203 (AIX: Implement TLS destructor guard in std and update dynamic-loading-cdylib test) - #165230 (Fix rigidity assertion ICE) - #165374 (Refactorings in preparation for the move trait)

JonathanBrouwer · 2026-11-03T14:15:04Z

Rollup failure #165403 (comment) probably caused by this PR?
@bors r-
@bors try jobs=test-aarch64-msvc-1

rust-bors · 2026-11-03T14:15:11Z

This pull request was unapproved.

View changes since this unapproval

Don't use `llvm_type` as the type of `static`s. try-job: test-aarch64-msvc-1

folkertdev · 2026-11-03T14:20:18Z

Yes seems likely. I wonder if we can make that test use minicore somehow?

beetrees · 2026-11-03T14:49:47Z

@bors try cancel

rust-bors · 2026-11-03T14:49:52Z

Try build cancelled. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/41651870820

Hint: if you want to run another try build, you do not need to manually cancel the previous one. Just run @bors try and bors will cancel the previous build automatically.

beetrees · 2026-11-03T14:50:04Z

@bors try jobs=test-aarch64-msvc-1

Don't use `llvm_type` as the type of `static`s. try-job: test-aarch64-msvc-1

rust-bors · 2026-11-03T17:23:36Z

☀️ Try build successful (CI)
Build commit: c904130 (c904130da12cc2415d08b1a52e8ad168630fdfba)
Base parent: 6a98084 (6a98084e4a21b04504c034dfc640e35e32ab9854)

beetrees · 2026-11-03T18:31:42Z

@bors r=folkertdev

rust-bors · 2026-11-03T18:31:45Z

📌 Commit bc082a3 has been approved by folkertdev

It is now in the queue for this repository.

…uwer Rollup of 7 pull requests Successful merges: - #165416 (fix unsoundness when compiling `[diverging_const; 0]`) - #163217 ([perf] rustdoc: Avoid re-HIR-ty-lowering tys & const args where possible) - #165371 (Don't use `llvm_type` as the type of `static`s.) - #165354 (Remove `AbiAlign`) - #165357 (rustdoc: Fix invalid clean of paths when they end with `self`) - #165401 (Add codegen test for step_by(1) loop vectorization) - #165408 (LLVMRustWriteOutputFile: Check the result of runCodeGenPipeline)

@folkertdev

Rollup merge of #165371 - beetrees:global-llvm-type, r=folkertdev Don't use `llvm_type` as the type of `static`s. This PR replaces the two uses of `llvm_type` as the type of `static`s - only the size of the static matters, so `cx.type_array(cx.type_i8(), layout.size)` is sufficient. As `static`s have to be redefined if the initialiser type is not the same as the existing type. using an LLVM `i8` array here is more optimal as it means the static type already matches the initializer in cases where the initializer is just plain bytes. r? @folkertdev

…uwer Rollup of 7 pull requests Successful merges: - rust-lang/rust#165416 (fix unsoundness when compiling `[diverging_const; 0]`) - rust-lang/rust#163217 ([perf] rustdoc: Avoid re-HIR-ty-lowering tys & const args where possible) - rust-lang/rust#165371 (Don't use `llvm_type` as the type of `static`s.) - rust-lang/rust#165354 (Remove `AbiAlign`) - rust-lang/rust#165357 (rustdoc: Fix invalid clean of paths when they end with `self`) - rust-lang/rust#165401 (Add codegen test for step_by(1) loop vectorization) - rust-lang/rust#165408 (LLVMRustWriteOutputFile: Check the result of runCodeGenPipeline)

rustbot assigned folkertdev Nov 2, 2026

beetrees force-pushed the global-llvm-type branch from bae9652 to 9b707b8 Compare November 2, 2026 20:07

folkertdev reviewed Nov 2, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Nov 2, 2026

Auto merge of #165371 - beetrees:global-llvm-type, r=<try>

6395ce3

Don't use `llvm_type` as the type of `static`s. try-job: test-x86_64-gnu-nopt

beetrees force-pushed the global-llvm-type branch from 9b707b8 to aff3637 Compare November 2, 2026 22:21

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 2, 2026

JonathanBrouwer mentioned this pull request Nov 3, 2026

Rollup of 14 pull requests #165392

Closed

JonathanBrouwer mentioned this pull request Nov 3, 2026

Rollup of 15 pull requests #165393

Closed

JonathanBrouwer mentioned this pull request Nov 3, 2026

Rollup of 14 pull requests #165394

Closed

JonathanBrouwer mentioned this pull request Nov 3, 2026

Rollup of 14 pull requests #165400

Closed

JonathanBrouwer mentioned this pull request Nov 3, 2026

Rollup of 13 pull requests #165403

Closed

rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 3, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Nov 3, 2026

Auto merge of #165371 - beetrees:global-llvm-type, r=<try>

0b5fa5b

Don't use `llvm_type` as the type of `static`s. try-job: test-aarch64-msvc-1

Don't use llvm_type as the type of statics.

bc082a3

beetrees force-pushed the global-llvm-type branch from aff3637 to bc082a3 Compare November 3, 2026 14:47

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Nov 3, 2026

Auto merge of #165371 - beetrees:global-llvm-type, r=<try>

c904130

Don't use `llvm_type` as the type of `static`s. try-job: test-aarch64-msvc-1

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 3, 2026

JonathanBrouwer mentioned this pull request Nov 3, 2026

Rollup of 7 pull requests #165432

Merged

rust-bors Bot merged commit c30b066 into rust-lang:main Nov 3, 2026
15 checks passed

rustbot added this to the 1.101.0 milestone Nov 3, 2026

beetrees deleted the global-llvm-type branch November 3, 2026 23:54

folkertdev mentioned this pull request Nov 4, 2026

add core::arch::{x86, x86_64}::x87_f80 #163992

Open

Conversation

beetrees commented Nov 2, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjorn3 commented Nov 2, 2026

Uh oh!

beetrees commented Nov 2, 2026

Uh oh!

folkertdev left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

folkertdev Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

beetrees Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

folkertdev Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

workingjubilee Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

beetrees Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beetrees Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

workingjubilee Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

beetrees commented Nov 2, 2026

Uh oh!

rust-bors Bot commented Nov 2, 2026

Uh oh!

rust-bors Bot commented Nov 2, 2026

Uh oh!

JonathanBrouwer commented Nov 3, 2026

Uh oh!

rust-bors Bot commented Nov 3, 2026

Uh oh!

This comment has been minimized.

folkertdev commented Nov 3, 2026

Uh oh!

beetrees commented Nov 3, 2026

Uh oh!

rust-bors Bot commented Nov 3, 2026

Uh oh!

beetrees commented Nov 3, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Nov 3, 2026

Uh oh!

beetrees commented Nov 3, 2026

Uh oh!

rust-bors Bot commented Nov 3, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

beetrees commented Nov 2, 2026 •

edited by rustbot

Loading

folkertdev left a comment •

edited by rustbot

Loading

beetrees Nov 2, 2026 •

edited

Loading

workingjubilee Nov 2, 2026 •

edited

Loading