Mention Register Size in `#[warn(asm_sub_register)]` #121940

veera-sivarajan · 2024-03-03T14:46:25Z

Displays the register size information obtained from suggest_modifier() and default_modifier().

rustbot · 2024-03-03T14:46:33Z

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

lcnr · 2024-03-04T14:44:35Z

r? compiler

Nadrieril · 2024-03-04T15:00:22Z

That looks very reasonable but I don't know this area

r? compiler

fmease

Thanks for working on this! Generally LGTM. I have minor suggestions / questions.

compiler/rustc_target/src/asm/mod.rs

fmease · 2024-03-12T21:24:38Z

compiler/rustc_target/src/asm/x86.rs

                _ => None,
            },
            Self::reg_byte => None,
            Self::xmm_reg => None,
            Self::ymm_reg => match ty.size().bits() {
                256 => None,
-                _ => Some(('x', "xmm0")),
+                _ => Some(('x', "xmm0", 128).into()),


Do you think it would be a good idea to turn the wildcard _ into 128 and to add an explicit _ => unreachable!() or bug!("unreachable")? Similarly for the other cases in this file?

Thanks for the suggestion.

But making this change panics on a few UI tests. It makes sense because the default size for any value using the YMM register class and less than 256 bits long is 128 bits.

Ah, yes! You are absolutely right 🤦

compiler/rustc_target/src/asm/aarch64.rs

fmease · 2024-03-14T02:02:00Z

@bors r+ rollup

bors · 2024-03-14T02:02:03Z

📌 Commit 1bde828 has been approved by fmease

It is now in the queue for this repository.

…mease Mention Register Size in `#[warn(asm_sub_register)]` Fixes rust-lang#121593 Displays the register size information obtained from `suggest_modifier()` and `default_modifier()`.

…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#104353 (Add CStr::bytes iterator) - rust-lang#114038 (unix time module now return result) - rust-lang#119676 (rustdoc-search: search types by higher-order functions) - rust-lang#120699 (Document `TRACK_DIAGNOSTIC` calls.) - rust-lang#121899 (Document how removing a type's field can be bad and what to do instead) - rust-lang#121940 (Mention Register Size in `#[warn(asm_sub_register)]`) - rust-lang#122397 (Various cleanups around the const eval query providers) - rust-lang#122405 (Add methods to create StableMIR constant) - rust-lang#122416 (Various style improvements to `rustc_lint::levels`) - rust-lang#122440 (const-eval: organize and extend tests for required-consts) - rust-lang#122461 (fix unsoundness in Step::forward_unchecked for signed integers) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2024-03-14T07:44:35Z

@bors r-
#122466 (comment)

fmease · 2024-03-15T01:01:49Z

@bors rollup=iffy

veera-sivarajan · 2024-03-20T23:40:11Z

@rustbot ready

…mease Mention Register Size in `#[warn(asm_sub_register)]` Fixes rust-lang#121593 Displays the register size information obtained from `suggest_modifier()` and `default_modifier()`.

…kingjubilee Rollup of 13 pull requests Successful merges: - rust-lang#121281 (regression test for rust-lang#103626) - rust-lang#121940 (Mention Register Size in `#[warn(asm_sub_register)]`) - rust-lang#122217 (Handle str literals written with `'` lexed as lifetime) - rust-lang#122379 (transmute: caution against int2ptr transmutation) - rust-lang#122460 (Rework rmake support library API) - rust-lang#122797 (Fix compile of wasm64-unknown-unknown target) - rust-lang#122875 (CFI: Support self_cell-like recursion) - rust-lang#122879 (CFI: Strip auto traits off Virtual calls) - rust-lang#122895 (add some ice tests 5xxxx to 9xxxx) - rust-lang#122907 (Uniquify `ReError` on input mode in canonicalizer) - rust-lang#122923 (In `pretty_print_type()`, print `async fn` futures' paths instead of spans.) - rust-lang#122942 (Add test in higher ranked subtype) - rust-lang#122963 (core/panicking: fix outdated comment) r? `@ghost` `@rustbot` modify labels: rollup

…mease Mention Register Size in `#[warn(asm_sub_register)]` Fixes rust-lang#121593 Displays the register size information obtained from `suggest_modifier()` and `default_modifier()`.

bors · 2024-03-24T04:40:17Z

⌛ Testing commit afc99cc with merge 8fee3f6...

Mention Register Size in `#[warn(asm_sub_register)]` Fixes rust-lang#121593 Displays the register size information obtained from `suggest_modifier()` and `default_modifier()`.

bors · 2024-03-24T04:42:21Z

💔 Test failed - checks-actions

rust-log-analyzer · 2024-03-24T04:47:25Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

workingjubilee · 2024-03-24T04:48:10Z

Looks like a network issue.

@bors retry

…mease Mention Register Size in `#[warn(asm_sub_register)]` Fixes rust-lang#121593 Displays the register size information obtained from `suggest_modifier()` and `default_modifier()`.

…kingjubilee Rollup of 7 pull requests Successful merges: - rust-lang#120419 (Expand sys/os for UEFI) - rust-lang#121940 (Mention Register Size in `#[warn(asm_sub_register)]`) - rust-lang#122762 (fix typo of endianness) - rust-lang#122797 (Fix compile of wasm64-unknown-unknown target) - rust-lang#122875 (CFI: Support self_cell-like recursion) - rust-lang#122879 (CFI: Strip auto traits off Virtual calls) - rust-lang#122969 (Simplify an iterator search in borrowck diag) r? `@ghost` `@rustbot` modify labels: rollup

bors · 2024-03-24T11:15:06Z

⌛ Testing commit afc99cc with merge 4a52e9c...

Rollup merge of rust-lang#121940 - veera-sivarajan:bugfix-121593, r=fmease Mention Register Size in `#[warn(asm_sub_register)]` Fixes rust-lang#121593 Displays the register size information obtained from `suggest_modifier()` and `default_modifier()`.

RalfJung · 2024-03-24T11:16:26Z

Thanks, this is great. :)

Would probably be good to have tests for these warnings as well.

workingjubilee · 2024-03-25T00:25:52Z

ones aside from the tests for the warnings that already exist? 🤔

RalfJung · 2024-03-25T07:12:49Z

Uh... I must have been blind when I looked at the PR diff as I saw no test changes. Please ignore me...

…mease Mention Register Size in `#[warn(asm_sub_register)]` Fixes rust-lang#121593 Displays the register size information obtained from `suggest_modifier()` and `default_modifier()`.

…kingjubilee Rollup of 7 pull requests Successful merges: - rust-lang#120419 (Expand sys/os for UEFI) - rust-lang#121940 (Mention Register Size in `#[warn(asm_sub_register)]`) - rust-lang#122762 (fix typo of endianness) - rust-lang#122797 (Fix compile of wasm64-unknown-unknown target) - rust-lang#122875 (CFI: Support self_cell-like recursion) - rust-lang#122879 (CFI: Strip auto traits off Virtual calls) - rust-lang#122969 (Simplify an iterator search in borrowck diag) r? `@ghost` `@rustbot` modify labels: rollup

…ifierinfo, r=petrochenkov Reduce Size of `ModifierInfo` I added `ModifierInfo` in rust-lang#121940 and had used a `u64` for the `size` field even though the largest value it holds is `512`. This PR changes the type of the `size` field to `u16`.

Rollup merge of rust-lang#123740 - veera-sivarajan:reduce-size-of-modifierinfo, r=petrochenkov Reduce Size of `ModifierInfo` I added `ModifierInfo` in rust-lang#121940 and had used a `u64` for the `size` field even though the largest value it holds is `512`. This PR changes the type of the `size` field to `u16`.

veera-sivarajan added 2 commits March 3, 2024 09:24

Update tests

d561a84

Mention Register Size in #[warn(asm_sub_register)]

9aac0c9

Fixes rust-lang#121593

rustbot assigned lcnr Mar 3, 2024

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 Mar 3, 2024

rustbot assigned Nadrieril and unassigned lcnr Mar 4, 2024

rustbot assigned fmease and unassigned Nadrieril Mar 4, 2024

fmease approved these changes Mar 12, 2024

View reviewed changes

fmease reviewed Mar 12, 2024

View reviewed changes

compiler/rustc_target/src/asm/aarch64.rs Show resolved Hide resolved

Improve style

1bde828

fmease approved these changes Mar 13, 2024

View reviewed changes

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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 14, 2024

matthiaskrgr mentioned this pull request Mar 14, 2024

Rollup of 11 pull requests #122466

Closed

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 Mar 14, 2024

Update test for aarch64

afc99cc

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 20, 2024

workingjubilee mentioned this pull request Mar 24, 2024

Rollup of 13 pull requests #122967

Closed

workingjubilee mentioned this pull request Mar 24, 2024

Rollup of 14 pull requests #122973

Closed

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

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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2024

This was referenced Mar 24, 2024

Rollup of 9 pull requests #122979

Closed

Rollup of 7 pull requests #122980

Merged

bors merged commit 97fcfaa into rust-lang:master Mar 24, 2024

rustbot added this to the 1.79.0 milestone Mar 24, 2024

veera-sivarajan mentioned this pull request Apr 10, 2024

Reduce Size of ModifierInfo #123740

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mention Register Size in `#[warn(asm_sub_register)]` #121940

Mention Register Size in `#[warn(asm_sub_register)]` #121940

veera-sivarajan commented Mar 3, 2024

rustbot commented Mar 3, 2024

lcnr commented Mar 4, 2024

Nadrieril commented Mar 4, 2024

fmease left a comment

fmease Mar 12, 2024 •

edited

Loading

veera-sivarajan Mar 13, 2024

fmease Mar 13, 2024

fmease commented Mar 14, 2024

bors commented Mar 14, 2024

matthiaskrgr commented Mar 14, 2024

fmease commented Mar 15, 2024

veera-sivarajan commented Mar 20, 2024

bors commented Mar 24, 2024

bors commented Mar 24, 2024

rust-log-analyzer commented Mar 24, 2024

workingjubilee commented Mar 24, 2024

bors commented Mar 24, 2024

RalfJung commented Mar 24, 2024

workingjubilee commented Mar 25, 2024

RalfJung commented Mar 25, 2024

Mention Register Size in #[warn(asm_sub_register)] #121940

Mention Register Size in #[warn(asm_sub_register)] #121940

Conversation

veera-sivarajan commented Mar 3, 2024

rustbot commented Mar 3, 2024

lcnr commented Mar 4, 2024

Nadrieril commented Mar 4, 2024

fmease left a comment

Choose a reason for hiding this comment

fmease Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

veera-sivarajan Mar 13, 2024

Choose a reason for hiding this comment

fmease Mar 13, 2024

Choose a reason for hiding this comment

fmease commented Mar 14, 2024

bors commented Mar 14, 2024

matthiaskrgr commented Mar 14, 2024

fmease commented Mar 15, 2024

veera-sivarajan commented Mar 20, 2024

bors commented Mar 24, 2024

bors commented Mar 24, 2024

rust-log-analyzer commented Mar 24, 2024

workingjubilee commented Mar 24, 2024

bors commented Mar 24, 2024

RalfJung commented Mar 24, 2024

workingjubilee commented Mar 25, 2024

RalfJung commented Mar 25, 2024

Mention Register Size in `#[warn(asm_sub_register)]` #121940

Mention Register Size in `#[warn(asm_sub_register)]` #121940

fmease Mar 12, 2024 •

edited

Loading