Skip to content

Fix target_feature handling in freg of LoongArch inline assembly #133587

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 30, 2024
Merged

Fix target_feature handling in freg of LoongArch inline assembly #133587

merged 1 commit into from
Nov 30, 2024

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Nov 28, 2024 •
edited
Loading

In LoongArch inline assembly, freg currently always accepts f32/f64 as input/output.

rust/compiler/rustc_target/src/asm/loongarch.rs

Line 41 in 9b4d7c6

Self::freg => types! { _: F32, F64; },

However, these types actually require f/d target features as in RISC-V.
Otherwise, an (ugly) compile error will occur: https://godbolt.org/z/K61Gq1E9E

f32/f64 without f:

error: couldn't allocate output register for constraint '{$f1}'
  --> <source>:12:11
   |
12 |     asm!("", in("$f1") x, lateout("$f1") y);
   |           ^

f64 with f but without d:

error: scalar-to-vector conversion failed, possible invalid constraint for vector type
  --> <source>:19:11
   |
19 |     asm!("", in("$f1") x, lateout("$f1") y);
   |           ^

cc @heiher

r? @Amanieu

@rustbot label +O-LoongArch +A-inline-assembly

@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. A-inline-assembly Area: Inline assembly (`asm!(…)`) O-loongarch Target: LoongArch (LA32R, LA32S, LA64) labels Nov 28, 2024
@Amanieu
Copy link
Member

Amanieu commented Nov 28, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 28, 2024

📌 Commit 0c8e36b has been approved by Amanieu

It is now in the queue for this repository.

@bors 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 Nov 28, 2024
@taiki-e taiki-e mentioned this pull request Nov 30, 2024
Closed
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Nov 30, 2024
@jieyouxu
Rollup merge of rust-lang#133587 - taiki-e:loongarch-asm-freg, r=Amanieu
470174d 
Fix target_feature handling in freg of LoongArch inline assembly

In LoongArch inline assembly, freg currently always accepts f32/f64 as input/output.

https://github.com/rust-lang/rust/blob/9b4d7c6a40b328d212095c28670c629facf1557d/compiler/rustc_target/src/asm/loongarch.rs#L41

However, these types actually require f/d target features as in RISC-V.
Otherwise, an (ugly) compile error will occur: https://godbolt.org/z/K61Gq1E9E

f32/f64 without f:

```
error: couldn't allocate output register for constraint '{$f1}'
  --> <source>:12:11
   |
12 |     asm!("", in("$f1") x, lateout("$f1") y);
   |           ^
```

f64 with f but without d:

```
error: scalar-to-vector conversion failed, possible invalid constraint for vector type
  --> <source>:19:11
   |
19 |     asm!("", in("$f1") x, lateout("$f1") y);
   |           ^
```

cc `@heiher`

r? `@Amanieu`

`@rustbot` label +O-LoongArch +A-inline-assembly
This was referenced Nov 30, 2024
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2024
@bors
Auto merge of rust-lang#133658 - jieyouxu:rollup-rq7e0gk, r=jieyouxu
b980396 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#116161 (Stabilize `extended_varargs_abi_support`)
 - rust-lang#132750 ([AIX] handle libunwind native_libs)
 - rust-lang#133488 (tests: Add regression test for self referential structs with cow as last field)
 - rust-lang#133569 (Bump `ruzstd` to 0.7.3)
 - rust-lang#133585 (Do not call `extern_crate` on current trait on crate mismatch errors)
 - rust-lang#133587 (Fix target_feature handling in freg of LoongArch inline assembly)
 - rust-lang#133599 (Add `+forced-atomics` feature to esp32s2 no_std  target)
 - rust-lang#133620 (Simplify hir_typeck_pass_to_variadic_function)
 - rust-lang#133623 (Improve span handling in `parse_expr_bottom`.)
 - rust-lang#133625 (custom MIR: add doc comment for debuginfo)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2024
@bors
Auto merge of rust-lang#133658 - jieyouxu:rollup-rq7e0gk, r=jieyouxu
e93e096 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#116161 (Stabilize `extended_varargs_abi_support`)
 - rust-lang#132750 ([AIX] handle libunwind native_libs)
 - rust-lang#133488 (tests: Add regression test for self referential structs with cow as last field)
 - rust-lang#133569 (Bump `ruzstd` to 0.7.3)
 - rust-lang#133585 (Do not call `extern_crate` on current trait on crate mismatch errors)
 - rust-lang#133587 (Fix target_feature handling in freg of LoongArch inline assembly)
 - rust-lang#133599 (Add `+forced-atomics` feature to esp32s2 no_std  target)
 - rust-lang#133620 (Simplify hir_typeck_pass_to_variadic_function)
 - rust-lang#133623 (Improve span handling in `parse_expr_bottom`.)
 - rust-lang#133625 (custom MIR: add doc comment for debuginfo)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ab4588a into rust-lang:master Nov 30, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Nov 30, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2024
@rust-timer
Unrolled build for rust-lang#133587
6fdace1 
Rollup merge of rust-lang#133587 - taiki-e:loongarch-asm-freg, r=Amanieu

Fix target_feature handling in freg of LoongArch inline assembly

In LoongArch inline assembly, freg currently always accepts f32/f64 as input/output.

https://github.com/rust-lang/rust/blob/9b4d7c6a40b328d212095c28670c629facf1557d/compiler/rustc_target/src/asm/loongarch.rs#L41

However, these types actually require f/d target features as in RISC-V.
Otherwise, an (ugly) compile error will occur: https://godbolt.org/z/K61Gq1E9E

f32/f64 without f:

```
error: couldn't allocate output register for constraint '{$f1}'
  --> <source>:12:11
   |
12 |     asm!("", in("$f1") x, lateout("$f1") y);
   |           ^
```

f64 with f but without d:

```
error: scalar-to-vector conversion failed, possible invalid constraint for vector type
  --> <source>:19:11
   |
19 |     asm!("", in("$f1") x, lateout("$f1") y);
   |           ^
```

cc ``@heiher``

r? ``@Amanieu``

``@rustbot`` label +O-LoongArch +A-inline-assembly
@taiki-e taiki-e deleted the loongarch-asm-freg branch November 30, 2024 17:26
@taiki-e taiki-e mentioned this pull request Dec 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

Labels
A-inline-assembly Area: Inline assembly (`asm!(…)`) O-loongarch Target: LoongArch (LA32R, LA32S, LA64) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.85.0
Development

Successfully merging this pull request may close these issues.
4 participants
@taiki-e @Amanieu @bors @rustbot