Rollup merge of rust-lang#131523 - nbdd0121:asm, r=compiler-errors

Fix asm goto with outputs and move it to a separate feature gate

Tracking issue: rust-lang#119364

This PR addresses 3 aspects of asm goto with outputs:
* Codegen is fixed. My initial implementation has an oversight which cause the output to be only stored in fallthrough path, but not in label blocks.
* Outputs can now be used with `options(noreturn)` if a label block is given.
* All of this is moved to a new feature gate, because we likely want to stabilise `asm_goto` before asm goto with outputs.

`@rustbot` labels: +A-inline-assembly +F-asm

Author: matthiaskrgr

compiler/rustc_ast_lowering/messages.ftl

@@ -181,6 +181,8 @@ ast_lowering_underscore_expr_lhs_assign =
     .label = `_` not allowed here
 
 ast_lowering_unstable_inline_assembly = inline assembly is not stable yet on this architecture
+ast_lowering_unstable_inline_assembly_label_operand_with_outputs =
+    using both label and output operands for inline assembly is unstable
 ast_lowering_unstable_inline_assembly_label_operands =
     label operands for inline assembly are unstable
 ast_lowering_unstable_may_unwind = the `may_unwind` option is unstable

compiler/rustc_ast_lowering/src/asm.rs

@@ -239,15 +239,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                         }
                     }
                     InlineAsmOperand::Label { block } => {
-                        if !self.tcx.features().asm_goto() {
-                            feature_err(
-                                sess,
-                                sym::asm_goto,
-                                *op_sp,
-                                fluent::ast_lowering_unstable_inline_assembly_label_operands,
-                            )
-                            .emit();
-                        }
                         hir::InlineAsmOperand::Label { block: self.lower_block(block, false) }
                     }
                 };
@@ -466,6 +457,41 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             }
         }
 
+        // Feature gate checking for asm goto.
+        if let Some((_, op_sp)) =
+            operands.iter().find(|(op, _)| matches!(op, hir::InlineAsmOperand::Label { .. }))
+        {
+            if !self.tcx.features().asm_goto() {
+                feature_err(
+                    sess,
+                    sym::asm_goto,
+                    *op_sp,
+                    fluent::ast_lowering_unstable_inline_assembly_label_operands,
+                )
+                .emit();
+            }
+
+            // In addition, check if an output operand is used.
+            // This is gated behind an additional feature.
+            let output_operand_used = operands.iter().any(|(op, _)| {
+                matches!(
+                    op,
+                    hir::InlineAsmOperand::Out { expr: Some(_), .. }
+                        | hir::InlineAsmOperand::InOut { .. }
+                        | hir::InlineAsmOperand::SplitInOut { out_expr: Some(_), .. }
+                )
+            });
+            if output_operand_used && !self.tcx.features().asm_goto_with_outputs() {
+                feature_err(
+                    sess,
+                    sym::asm_goto_with_outputs,
+                    *op_sp,
+                    fluent::ast_lowering_unstable_inline_assembly_label_operand_with_outputs,
+                )
+                .emit();
+            }
+        }
+
         let operands = self.arena.alloc_from_iter(operands);
         let template = self.arena.alloc_from_iter(asm.template.iter().cloned());
         let template_strs = self.arena.alloc_from_iter(

compiler/rustc_builtin_macros/src/asm.rs

@@ -300,7 +300,10 @@ pub fn parse_asm_args<'a>(
     if args.options.contains(ast::InlineAsmOptions::PURE) && !have_real_output {
         dcx.emit_err(errors::AsmPureNoOutput { spans: args.options_spans.clone() });
     }
-    if args.options.contains(ast::InlineAsmOptions::NORETURN) && !outputs_sp.is_empty() {
+    if args.options.contains(ast::InlineAsmOptions::NORETURN)
+        && !outputs_sp.is_empty()
+        && labels_sp.is_empty()
+    {
         let err = dcx.create_err(errors::AsmNoReturn { outputs_sp });
         // Bail out now since this is likely to confuse MIR
         return Err(err);

compiler/rustc_codegen_llvm/src/asm.rs

@@ -342,24 +342,32 @@ impl<'ll, 'tcx> AsmBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
         }
         attributes::apply_to_callsite(result, llvm::AttributePlace::Function, &{ attrs });
 
-        // Switch to the 'normal' basic block if we did an `invoke` instead of a `call`
-        if let Some(dest) = dest {
-            self.switch_to_block(dest);
-        }
+        // Write results to outputs. We need to do this for all possible control flow.
+        //
+        // Note that `dest` maybe populated with unreachable_block when asm goto with outputs
+        // is used (because we need to codegen callbr which always needs a destination), so
+        // here we use the NORETURN option to determine if `dest` should be used.
+        for block in (if options.contains(InlineAsmOptions::NORETURN) { None } else { Some(dest) })
+            .into_iter()
+            .chain(labels.iter().copied().map(Some))
+        {
+            if let Some(block) = block {
+                self.switch_to_block(block);
+            }
 
-        // Write results to outputs
-        for (idx, op) in operands.iter().enumerate() {
-            if let InlineAsmOperandRef::Out { reg, place: Some(place), .. }
-            | InlineAsmOperandRef::InOut { reg, out_place: Some(place), .. } = *op
-            {
-                let value = if output_types.len() == 1 {
-                    result
-                } else {
-                    self.extract_value(result, op_idx[&idx] as u64)
-                };
-                let value =
-                    llvm_fixup_output(self, value, reg.reg_class(), &place.layout, instance);
-                OperandValue::Immediate(value).store(self, place);
+            for (idx, op) in operands.iter().enumerate() {
+                if let InlineAsmOperandRef::Out { reg, place: Some(place), .. }
+                | InlineAsmOperandRef::InOut { reg, out_place: Some(place), .. } = *op
+                {
+                    let value = if output_types.len() == 1 {
+                        result
+                    } else {
+                        self.extract_value(result, op_idx[&idx] as u64)
+                    };
+                    let value =
+                        llvm_fixup_output(self, value, reg.reg_class(), &place.layout, instance);
+                    OperandValue::Immediate(value).store(self, place);
+                }
             }
         }
     }

compiler/rustc_feature/src/unstable.rs

@@ -378,6 +378,8 @@ declare_features! (
     (unstable, asm_experimental_arch, "1.58.0", Some(93335)),
     /// Allows using `label` operands in inline assembly.
     (unstable, asm_goto, "1.78.0", Some(119364)),
+    /// Allows using `label` operands in inline assembly together with output operands.
+    (unstable, asm_goto_with_outputs, "CURRENT_RUSTC_VERSION", Some(119364)),
     /// Allows the `may_unwind` option in inline assembly.
     (unstable, asm_unwind, "1.58.0", Some(93334)),
     /// Allows users to enforce equality of associated constants `TraitImpl<AssocConst=3>`.

compiler/rustc_span/src/symbol.rs

@@ -417,6 +417,7 @@ symbols! {
         asm_const,
         asm_experimental_arch,
         asm_goto,
+        asm_goto_with_outputs,
         asm_sym,
         asm_unwind,
         assert,

tests/codegen/asm/goto.rs

@@ -2,21 +2,10 @@
 //@ only-x86_64
 
 #![crate_type = "rlib"]
-#![feature(asm_goto)]
+#![feature(asm_goto, asm_goto_with_outputs)]
 
 use std::arch::asm;
 
-#[no_mangle]
-pub extern "C" fn panicky() {}
-
-struct Foo;
-
-impl Drop for Foo {
-    fn drop(&mut self) {
-        println!();
-    }
-}
-
 // CHECK-LABEL: @asm_goto
 #[no_mangle]
 pub unsafe fn asm_goto() {
@@ -38,14 +27,37 @@ pub unsafe fn asm_goto_with_outputs() -> u64 {
     out
 }
 
+// CHECK-LABEL: @asm_goto_with_outputs_use_in_label
+#[no_mangle]
+pub unsafe fn asm_goto_with_outputs_use_in_label() -> u64 {
+    let out: u64;
+    // CHECK: [[RES:%[0-9]+]] = callbr i64 asm sideeffect alignstack inteldialect "
+    // CHECK-NEXT: to label %[[FALLTHROUGHBB:[a-b0-9]+]] [label %[[JUMPBB:[a-b0-9]+]]]
+    asm!("{} /* {} */", out(reg) out, label { return out; });
+    // CHECK: [[JUMPBB]]:
+    // CHECK-NEXT: [[RET:%.+]] = phi i64 [ 1, %[[FALLTHROUGHBB]] ], [ [[RES]], %start ]
+    // CHECK-NEXT: ret i64 [[RET]]
+    1
+}
+
 // CHECK-LABEL: @asm_goto_noreturn
 #[no_mangle]
 pub unsafe fn asm_goto_noreturn() -> u64 {
-    let out: u64;
     // CHECK: callbr void asm sideeffect alignstack inteldialect "
     // CHECK-NEXT: to label %unreachable [label %[[JUMPBB:[a-b0-9]+]]]
     asm!("jmp {}", label { return 1; }, options(noreturn));
     // CHECK: [[JUMPBB]]:
     // CHECK-NEXT: ret i64 1
+}
+
+// CHECK-LABEL: @asm_goto_noreturn_with_outputs
+#[no_mangle]
+pub unsafe fn asm_goto_noreturn_with_outputs() -> u64 {
+    let out: u64;
+    // CHECK: [[RES:%[0-9]+]] = callbr i64 asm sideeffect alignstack inteldialect "
+    // CHECK-NEXT: to label %[[FALLTHROUGHBB:[a-b0-9]+]] [label %[[JUMPBB:[a-b0-9]+]]]
+    asm!("mov {}, 1", "jmp {}", out(reg) out, label { return out; });
+    // CHECK: [[JUMPBB]]:
+    // CHECK-NEXT: ret i64 [[RES]]
     out
 }

tests/ui/asm/x86_64/goto.rs

@@ -3,7 +3,7 @@
 //@ needs-asm-support
 
 #![deny(unreachable_code)]
-#![feature(asm_goto)]
+#![feature(asm_goto, asm_goto_with_outputs)]
 
 use std::arch::asm;
 
@@ -31,10 +31,6 @@ fn goto_jump() {
     }
 }
 
-// asm goto with outputs cause miscompilation in LLVM. UB can be triggered
-// when outputs are used inside the label block when optimisation is enabled.
-// See: https://github.com/llvm/llvm-project/issues/74483
-/*
 fn goto_out_fallthrough() {
     unsafe {
         let mut out: usize;
@@ -68,7 +64,57 @@ fn goto_out_jump() {
         assert!(value);
     }
 }
-*/
+
+fn goto_out_jump_noreturn() {
+    unsafe {
+        let mut value = false;
+        let mut out: usize;
+        asm!(
+            "lea {}, [{} + 1]",
+            "jmp {}",
+            out(reg) out,
+            in(reg) 0x12345678usize,
+            label {
+                value = true;
+                assert_eq!(out, 0x12345679);
+            },
+            options(noreturn)
+        );
+        assert!(value);
+    }
+}
+
+// asm goto with outputs cause miscompilation in LLVM when multiple outputs are present.
+// The code sample below is adapted from https://github.com/llvm/llvm-project/issues/74483
+// and does not work with `-C opt-level=0`
+#[expect(unused)]
+fn goto_multi_out() {
+    #[inline(never)]
+    #[allow(unused)]
+    fn goto_multi_out(a: usize, b: usize) -> usize {
+        let mut x: usize;
+        let mut y: usize;
+        let mut z: usize;
+        unsafe {
+            core::arch::asm!(
+                "mov {x}, {a}",
+                "test {a}, {a}",
+                "jnz {label1}",
+                "/* {y} {z} {b} {label2} */",
+                x = out(reg) x,
+                y = out(reg) y,
+                z = out(reg) z,
+                a = in(reg) a,
+                b = in(reg) b,
+                label1 = label { return x },
+                label2 = label { return 1 },
+            );
+            0
+        }
+    }
+
+    assert_eq!(goto_multi_out(11, 22), 11);
+}
 
 fn goto_noreturn() {
     unsafe {
@@ -102,8 +148,10 @@ fn goto_noreturn_diverge() {
 fn main() {
     goto_fallthough();
     goto_jump();
-    // goto_out_fallthrough();
-    // goto_out_jump();
+    goto_out_fallthrough();
+    goto_out_jump();
+    goto_out_jump_noreturn();
+    // goto_multi_out();
     goto_noreturn();
     goto_noreturn_diverge();
 }

tests/ui/asm/x86_64/goto.stderr

@@ -1,5 +1,5 @@
 warning: unreachable statement
-  --> $DIR/goto.rs:97:9
+  --> $DIR/goto.rs:143:9
    |
 LL | /         asm!(
 LL | |             "jmp {}",
@@ -13,7 +13,7 @@ LL |           unreachable!();
    |           ^^^^^^^^^^^^^^ unreachable statement
    |
 note: the lint level is defined here
-  --> $DIR/goto.rs:87:8
+  --> $DIR/goto.rs:133:8
    |
 LL | #[warn(unreachable_code)]
    |        ^^^^^^^^^^^^^^^^

tests/ui/feature-gates/feature-gate-asm_goto_with_outputs.rs

@@ -0,0 +1,13 @@
+//@ only-x86_64
+
+#![feature(asm_goto)]
+
+use std::arch::asm;
+
+fn main() {
+    let mut _out: u64;
+    unsafe {
+        asm!("mov {}, 1", "jmp {}", out(reg) _out, label {});
+        //~^ ERROR using both label and output operands for inline assembly is unstable
+    }
+}

tests/ui/feature-gates/feature-gate-asm_goto_with_outputs.stderr

@@ -0,0 +1,13 @@
+error[E0658]: using both label and output operands for inline assembly is unstable
+  --> $DIR/feature-gate-asm_goto_with_outputs.rs:10:52
+   |
+LL |         asm!("mov {}, 1", "jmp {}", out(reg) _out, label {});
+   |                                                    ^^^^^^^^
+   |
+   = note: see issue #119364 <https://github.com/rust-lang/rust/issues/119364> for more information
+   = help: add `#![feature(asm_goto_with_outputs)]` to the crate attributes to enable
+   = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0658`.