Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi` #141569

workingjubilee · 2025-05-26T05:40:28Z

Our conv_from_spec_abi, adjust_abi, and is_abi_supported combine to give us a very confusing way of reasoning about what actual calling convention we want to lower our code to and whether we want to compile the resulting code at all. Instead of leaving this code as a miniature adventure game in which someone tries to combine stateful mutations into a Rube Goldberg machine that will let them escape the maze and arrive at the promised land of codegen, we let AbiMap devour this complexity. Once you have an AbiMap, you can answer which ExternAbis will lower to what CanonAbis (and whether they will lower at all).

Removed:

conv_from_spec_abi replaced by AbiMap::canonize_abi
adjust_abi replaced by same
Conv::PreserveAll as unused
Conv::Cold as unused
enum Conv replaced by enum CanonAbi

target-spec.json changes:

If you have a target-spec.json then now your "entry-abi" key will be specified in terms of one of the "{abi}" strings Rust recognizes, e.g.

    "entry-abi": "C",
    "entry-abi": "win64",
    "entry-abi": "aapcs",

workingjubilee · 2025-05-26T05:44:14Z

I'd like to see you
@bors try

Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi` I am having second thoughts about some of my design choices, here, but I think it's useless to _internally_ debate them further. r? `@ghost` try-job: test-various try-job: aarch64-apple try-job: aarch64-gnu try-job: armhf-gnu try-job: x86_64-msvc-1 try-job: x86_64-msvc-2 try-job: x86_64-mingw-1 try-job: x86_64-mingw-2 try-job: i686-msvc-1 try-job: i686-msvc-2

workingjubilee · 2025-05-26T06:22:31Z

I am foolish and impatient.
@bors try-
@bors try

bors · 2025-05-26T06:23:41Z

⌛ Trying commit e9817cc with merge 9938b8a...

Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi` I am having second thoughts about some of my design choices, here, but I think it's useless to _internally_ debate them further. r? `@ghost` try-job: test-various try-job: aarch64-apple try-job: aarch64-gnu try-job: armhf-gnu try-job: x86_64-msvc-1 try-job: x86_64-msvc-2 try-job: x86_64-mingw-1 try-job: x86_64-mingw-2 try-job: i686-msvc-1 try-job: i686-msvc-2

compiler/rustc_abi/src/map.rs

bjorn3 · 2025-05-26T09:19:17Z

compiler/rustc_abi/src/canon_abi.rs

+
+    /// ABIs relevant to Windows or x86 targets
+    X86(X86Call),
+}


Ah so that decision actually does look right to someone who is not inside my head? Great.

compiler/rustc_target/src/spec/targets/i686_pc_windows_msvc.rs

workingjubilee · 2025-05-26T18:49:32Z

@bors try

bors · 2025-05-26T18:50:44Z

⌛ Trying commit 17fab7d with merge c4f8500...

Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi` I am having second thoughts about some of my design choices, here, but I think it's useless to _internally_ debate them further. r? `@ghost` try-job: test-various try-job: aarch64-apple try-job: aarch64-gnu try-job: armhf-gnu try-job: x86_64-msvc-1 try-job: x86_64-msvc-2 try-job: x86_64-mingw-1 try-job: x86_64-mingw-2 try-job: i686-msvc-1 try-job: i686-msvc-2

makes entry_abi a lowering of the ABI string, so now it can be ```json "entry_abi": "C", "entry_abi": "win64", "entry_abi": "aapcs", ```

`adjust_abi` is not needed and `is_abi_supported` can be a 1-liner.

workingjubilee · 2025-06-03T17:26:12Z

@bors r-

workingjubilee · 2025-06-03T18:34:29Z

implemented requested namechange by Ralf in this diff and this is passing CI again. I think Ralf is traveling or sommat and this is otherwise functionally identical so

@bors r=bjorn3

bors · 2025-06-03T18:34:31Z

📌 Commit 307a18d has been approved by bjorn3

It is now in the queue for this repository.

Rollup of 8 pull requests Successful merges: - #137725 (Add `iter` macro) - #141455 (std: abort the process on failure to allocate a TLS key) - #141569 (Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi`) - #141698 (Use the informative error as the main const eval error message) - #141925 (Remove bootstrap cfgs from library/) - #141943 (Remove pre-expansion AST stats.) - #141945 (Remove `Path::is_ident`.) - #141957 (Add missing `dyn` keywords to tests that do not test for them Part 2) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #141569 - workingjubilee:canonicalize-abi, r=bjorn3 Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi` Our `conv_from_spec_abi`, `adjust_abi`, and `is_abi_supported` combine to give us a very confusing way of reasoning about what _actual_ calling convention we want to lower our code to and whether we want to compile the resulting code at all. Instead of leaving this code as a miniature adventure game in which someone tries to combine stateful mutations into a Rube Goldberg machine that will let them escape the maze and arrive at the promised land of codegen, we let `AbiMap` devour this complexity. Once you have an `AbiMap`, you can answer which `ExternAbi`s will lower to what `CanonAbi`s (and whether they will lower at all). Removed: - `conv_from_spec_abi` replaced by `AbiMap::canonize_abi` - `adjust_abi` replaced by same - `Conv::PreserveAll` as unused - `Conv::Cold` as unused - `enum Conv` replaced by `enum CanonAbi` target-spec.json changes: - If you have a target-spec.json then now your "entry-abi" key will be specified in terms of one of the `"{abi}"` strings Rust recognizes, e.g. ```json "entry-abi": "C", "entry-abi": "win64", "entry-abi": "aapcs", ```

Rollup of 8 pull requests Successful merges: - rust-lang/rust#137725 (Add `iter` macro) - rust-lang/rust#141455 (std: abort the process on failure to allocate a TLS key) - rust-lang/rust#141569 (Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi`) - rust-lang/rust#141698 (Use the informative error as the main const eval error message) - rust-lang/rust#141925 (Remove bootstrap cfgs from library/) - rust-lang/rust#141943 (Remove pre-expansion AST stats.) - rust-lang/rust#141945 (Remove `Path::is_ident`.) - rust-lang/rust#141957 (Add missing `dyn` keywords to tests that do not test for them Part 2) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#137725 (Add `iter` macro) - rust-lang#141455 (std: abort the process on failure to allocate a TLS key) - rust-lang#141569 (Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi`) - rust-lang#141698 (Use the informative error as the main const eval error message) - rust-lang#141925 (Remove bootstrap cfgs from library/) - rust-lang#141943 (Remove pre-expansion AST stats.) - rust-lang#141945 (Remove `Path::is_ident`.) - rust-lang#141957 (Add missing `dyn` keywords to tests that do not test for them Part 2) r? `@ghost` `@rustbot` modify labels: rollup

…=bjorn3 Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi` Our `conv_from_spec_abi`, `adjust_abi`, and `is_abi_supported` combine to give us a very confusing way of reasoning about what _actual_ calling convention we want to lower our code to and whether we want to compile the resulting code at all. Instead of leaving this code as a miniature adventure game in which someone tries to combine stateful mutations into a Rube Goldberg machine that will let them escape the maze and arrive at the promised land of codegen, we let `AbiMap` devour this complexity. Once you have an `AbiMap`, you can answer which `ExternAbi`s will lower to what `CanonAbi`s (and whether they will lower at all). Removed: - `conv_from_spec_abi` replaced by `AbiMap::canonize_abi` - `adjust_abi` replaced by same - `Conv::PreserveAll` as unused - `Conv::Cold` as unused - `enum Conv` replaced by `enum CanonAbi` target-spec.json changes: - If you have a target-spec.json then now your "entry-abi" key will be specified in terms of one of the `"{abi}"` strings Rust recognizes, e.g. ```json "entry-abi": "C", "entry-abi": "win64", "entry-abi": "aapcs", ```

rustbot added O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) PG-exploit-mitigations Project group: Exploit mitigations T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 26, 2025

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

workingjubilee force-pushed the canonicalize-abi branch from 7e6762a to 71abc30 Compare May 26, 2025 06:06

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

workingjubilee force-pushed the canonicalize-abi branch from 71abc30 to e9817cc Compare May 26, 2025 06:21

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

bjorn3 reviewed May 26, 2025

View reviewed changes

compiler/rustc_abi/src/map.rs Outdated Show resolved Hide resolved

bjorn3 reviewed May 26, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

bjorn3 reviewed May 26, 2025

View reviewed changes

compiler/rustc_target/src/spec/targets/i686_pc_windows_msvc.rs Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

workingjubilee force-pushed the canonicalize-abi branch from 180ed85 to 17fab7d Compare May 26, 2025 18:46

workingjubilee removed PG-exploit-mitigations Project group: Exploit mitigations O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) labels May 26, 2025

workingjubilee added 9 commits June 3, 2025 10:02

compiler: use CanonAbi for entry_abi

2d637f7

makes entry_abi a lowering of the ABI string, so now it can be ```json "entry_abi": "C", "entry_abi": "win64", "entry_abi": "aapcs", ```

compiler: change Conv to CanonAbi

72ecde2

cg_llvm: convert to CanonAbi

e0b07a8

cg_clif: convert to CanonAbi

0d8959f

cg_gcc: convert to CanonAbi

87fa1ea

miri: sed to CanonAbi

9d42e35

bless test for CanonAbi::X86(SysV64) change

2ad64b4

compiler: simplify TargetOptions ABI functions

2351a3e

`adjust_abi` is not needed and `is_abi_supported` can be a 1-liner.

compiler: actually remove Conv now that it is irrelevant

307a18d

workingjubilee force-pushed the canonicalize-abi branch from 8eebba0 to 307a18d Compare June 3, 2025 17:25

bors 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 Jun 3, 2025

bors 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 Jun 3, 2025

matthiaskrgr mentioned this pull request Jun 3, 2025

Rollup of 8 pull requests #141984

Merged

bors merged commit 644f06e into rust-lang:master Jun 4, 2025
10 checks passed

rustbot added this to the 1.89.0 milestone Jun 4, 2025

workingjubilee deleted the canonicalize-abi branch June 4, 2025 03:31

matthiaskrgr mentioned this pull request Jun 6, 2025

ICE: abi_map: None #142107

Closed

RalfJung mentioned this pull request Jun 12, 2025

Tracking Issue for extern_system_varargs #136946

Open

4 tasks

matthiaskrgr mentioned this pull request Jun 24, 2025

ICE None abi map #142969

Closed

Replace ad-hoc ABI "adjustments" with an AbiMap to CanonAbi #141569

Replace ad-hoc ABI "adjustments" with an AbiMap to CanonAbi #141569

Uh oh!

Conversation

workingjubilee commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented May 26, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as outdated.

workingjubilee commented May 26, 2025

Uh oh!

bors commented May 26, 2025

Uh oh!

This comment has been minimized.

This comment was marked as outdated.

This comment has been minimized.

Uh oh!

bjorn3 May 26, 2025

Choose a reason for hiding this comment

Uh oh!

workingjubilee May 26, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

workingjubilee commented May 26, 2025

Uh oh!

bors commented May 26, 2025

Uh oh!

workingjubilee commented Jun 3, 2025

Uh oh!

workingjubilee commented Jun 3, 2025

Uh oh!

bors commented Jun 3, 2025

Uh oh!

Uh oh!

Uh oh!

Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi` #141569

Replace ad-hoc ABI "adjustments" with an `AbiMap` to `CanonAbi` #141569

workingjubilee commented May 26, 2025 •

edited

Loading