Rollup merge of #89641 - asquared31415:asm-feature-attr-regs, r=oli-obk

make #[target_feature] work with `asm` register classes

Fixes #89289

compiler/rustc_ast_lowering/src/asm.rs

@@ -202,39 +202,20 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
         let mut used_input_regs = FxHashMap::default();
         let mut used_output_regs = FxHashMap::default();
-        let mut required_features: Vec<&str> = vec![];
+
         for (idx, &(ref op, op_sp)) in operands.iter().enumerate() {
             if let Some(reg) = op.reg() {
-                // Make sure we don't accidentally carry features from the
-                // previous iteration.
-                required_features.clear();
-
                 let reg_class = reg.reg_class();
                 if reg_class == asm::InlineAsmRegClass::Err {
                     continue;
                 }
 
-                // We ignore target feature requirements for clobbers: if the
-                // feature is disabled then the compiler doesn't care what we
-                // do with the registers.
-                //
-                // Note that this is only possible for explicit register
-                // operands, which cannot be used in the asm string.
-                let is_clobber = matches!(
-                    op,
-                    hir::InlineAsmOperand::Out {
-                        reg: asm::InlineAsmRegOrRegClass::Reg(_),
-                        late: _,
-                        expr: None
-                    }
-                );
-
                 // Some register classes can only be used as clobbers. This
                 // means that we disallow passing a value in/out of the asm and
                 // require that the operand name an explicit register, not a
                 // register class.
                 if reg_class.is_clobber_only(asm_arch.unwrap())
-                    && !(is_clobber && matches!(reg, asm::InlineAsmRegOrRegClass::Reg(_)))
+                    && !(op.is_clobber() && matches!(reg, asm::InlineAsmRegOrRegClass::Reg(_)))
                 {
                     let msg = format!(
                         "register class `{}` can only be used as a clobber, \
@@ -245,47 +226,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                     continue;
                 }
 
-                if !is_clobber {
-                    // Validate register classes against currently enabled target
-                    // features. We check that at least one type is available for
-                    // the current target.
-                    for &(_, feature) in reg_class.supported_types(asm_arch.unwrap()) {
-                        if let Some(feature) = feature {
-                            if self.sess.target_features.contains(&Symbol::intern(feature)) {
-                                required_features.clear();
-                                break;
-                            } else {
-                                required_features.push(feature);
-                            }
-                        } else {
-                            required_features.clear();
-                            break;
-                        }
-                    }
-                    // We are sorting primitive strs here and can use unstable sort here
-                    required_features.sort_unstable();
-                    required_features.dedup();
-                    match &required_features[..] {
-                        [] => {}
-                        [feature] => {
-                            let msg = format!(
-                                "register class `{}` requires the `{}` target feature",
-                                reg_class.name(),
-                                feature
-                            );
-                            sess.struct_span_err(op_sp, &msg).emit();
-                        }
-                        features => {
-                            let msg = format!(
-                                "register class `{}` requires at least one target feature: {}",
-                                reg_class.name(),
-                                features.join(", ")
-                            );
-                            sess.struct_span_err(op_sp, &msg).emit();
-                        }
-                    }
-                }
-
                 // Check for conflicts between explicit register operands.
                 if let asm::InlineAsmRegOrRegClass::Reg(reg) = reg {
                     let (input, output) = match op {

compiler/rustc_hir/src/hir.rs

@@ -2293,6 +2293,13 @@ impl<'hir> InlineAsmOperand<'hir> {
             Self::Const { .. } | Self::Sym { .. } => None,
         }
     }
+
+    pub fn is_clobber(&self) -> bool {
+        matches!(
+            self,
+            InlineAsmOperand::Out { reg: InlineAsmRegOrRegClass::Reg(_), late: _, expr: None }
+        )
+    }
 }
 
 #[derive(Debug, HashStable_Generic)]

compiler/rustc_passes/src/intrinsicck.rs

@@ -141,6 +141,7 @@ impl ExprVisitor<'tcx> {
         template: &[InlineAsmTemplatePiece],
         is_input: bool,
         tied_input: Option<(&hir::Expr<'tcx>, Option<InlineAsmType>)>,
+        target_features: &[Symbol],
     ) -> Option<InlineAsmType> {
         // Check the type against the allowed types for inline asm.
         let ty = self.typeck_results.expr_ty_adjusted(expr);
@@ -283,17 +284,20 @@ impl ExprVisitor<'tcx> {
         };
 
         // Check whether the selected type requires a target feature. Note that
-        // this is different from the feature check we did earlier in AST
+        // this is different from the feature check we did earlier. While the
-        // lowering. While AST lowering checked that this register class is
+        // previous check checked that this register class is usable at all
-        // usable at all with the currently enabled features, some types may
+        // with the currently enabled features, some types may only be usable
-        // only be usable with a register class when a certain feature is
+        // with a register class when a certain feature is enabled. We check
-        // enabled. We check this here since it depends on the results of typeck.
+        // this here since it depends on the results of typeck.
         //
         // Also note that this check isn't run when the operand type is never
-        // (!). In that case we still need the earlier check in AST lowering to
+        // (!). In that case we still need the earlier check to verify that the
-        // verify that the register class is usable at all.
+        // register class is usable at all.
         if let Some(feature) = feature {
-            if !self.tcx.sess.target_features.contains(&Symbol::intern(feature)) {
+            let feat_sym = Symbol::intern(feature);
+            if !self.tcx.sess.target_features.contains(&feat_sym)
+                && !target_features.contains(&feat_sym)
+            {
                 let msg = &format!("`{}` target feature is not enabled", feature);
                 let mut err = self.tcx.sess.struct_span_err(expr.span, msg);
                 err.note(&format!(
@@ -349,23 +353,122 @@ impl ExprVisitor<'tcx> {
         Some(asm_ty)
     }
 
-    fn check_asm(&self, asm: &hir::InlineAsm<'tcx>) {
+    fn check_asm(&self, asm: &hir::InlineAsm<'tcx>, hir_id: hir::HirId) {
-        for (idx, (op, _)) in asm.operands.iter().enumerate() {
+        let hir = self.tcx.hir();
+        let enclosing_id = hir.enclosing_body_owner(hir_id);
+        let enclosing_def_id = hir.local_def_id(enclosing_id).to_def_id();
+        let attrs = self.tcx.codegen_fn_attrs(enclosing_def_id);
+        for (idx, (op, op_sp)) in asm.operands.iter().enumerate() {
+            // Validate register classes against currently enabled target
+            // features. We check that at least one type is available for
+            // the enabled features.
+            //
+            // We ignore target feature requirements for clobbers: if the
+            // feature is disabled then the compiler doesn't care what we
+            // do with the registers.
+            //
+            // Note that this is only possible for explicit register
+            // operands, which cannot be used in the asm string.
+            if let Some(reg) = op.reg() {
+                if !op.is_clobber() {
+                    let mut missing_required_features = vec![];
+                    let reg_class = reg.reg_class();
+                    for &(_, feature) in reg_class.supported_types(self.tcx.sess.asm_arch.unwrap())
+                    {
+                        match feature {
+                            Some(feature) => {
+                                let feat_sym = Symbol::intern(feature);
+                                if self.tcx.sess.target_features.contains(&feat_sym)
+                                    || attrs.target_features.contains(&feat_sym)
+                                {
+                                    missing_required_features.clear();
+                                    break;
+                                } else {
+                                    missing_required_features.push(feature);
+                                }
+                            }
+                            None => {
+                                missing_required_features.clear();
+                                break;
+                            }
+                        }
+                    }
+
+                    // We are sorting primitive strs here and can use unstable sort here
+                    missing_required_features.sort_unstable();
+                    missing_required_features.dedup();
+                    match &missing_required_features[..] {
+                        [] => {}
+                        [feature] => {
+                            let msg = format!(
+                                "register class `{}` requires the `{}` target feature",
+                                reg_class.name(),
+                                feature
+                            );
+                            self.tcx.sess.struct_span_err(*op_sp, &msg).emit();
+                            // register isn't enabled, don't do more checks
+                            continue;
+                        }
+                        features => {
+                            let msg = format!(
+                                "register class `{}` requires at least one of the following target features: {}",
+                                reg_class.name(),
+                                features.join(", ")
+                            );
+                            self.tcx.sess.struct_span_err(*op_sp, &msg).emit();
+                            // register isn't enabled, don't do more checks
+                            continue;
+                        }
+                    }
+                }
+            }
+
             match *op {
                 hir::InlineAsmOperand::In { reg, ref expr } => {
-                    self.check_asm_operand_type(idx, reg, expr, asm.template, true, None);
+                    self.check_asm_operand_type(
+                        idx,
+                        reg,
+                        expr,
+                        asm.template,
+                        true,
+                        None,
+                        &attrs.target_features,
+                    );
                 }
                 hir::InlineAsmOperand::Out { reg, late: _, ref expr } => {
                     if let Some(expr) = expr {
-                        self.check_asm_operand_type(idx, reg, expr, asm.template, false, None);
+                        self.check_asm_operand_type(
+                            idx,
+                            reg,
+                            expr,
+                            asm.template,
+                            false,
+                            None,
+                            &attrs.target_features,
+                        );
                     }
                 }
                 hir::InlineAsmOperand::InOut { reg, late: _, ref expr } => {
-                    self.check_asm_operand_type(idx, reg, expr, asm.template, false, None);
+                    self.check_asm_operand_type(
+                        idx,
+                        reg,
+                        expr,
+                        asm.template,
+                        false,
+                        None,
+                        &attrs.target_features,
+                    );
                 }
                 hir::InlineAsmOperand::SplitInOut { reg, late: _, ref in_expr, ref out_expr } => {
-                    let in_ty =
+                    let in_ty = self.check_asm_operand_type(
-                        self.check_asm_operand_type(idx, reg, in_expr, asm.template, true, None);
+                        idx,
+                        reg,
+                        in_expr,
+                        asm.template,
+                        true,
+                        None,
+                        &attrs.target_features,
+                    );
                     if let Some(out_expr) = out_expr {
                         self.check_asm_operand_type(
                             idx,
@@ -374,6 +477,7 @@ impl ExprVisitor<'tcx> {
                             asm.template,
                             false,
                             Some((in_expr, in_ty)),
+                            &attrs.target_features,
                         );
                     }
                 }
@@ -422,7 +526,7 @@ impl Visitor<'tcx> for ExprVisitor<'tcx> {
                 }
             }
 
-            hir::ExprKind::InlineAsm(asm) => self.check_asm(asm),
+            hir::ExprKind::InlineAsm(asm) => self.check_asm(asm, expr.hir_id),
 
             _ => {}
         }

src/test/ui/asm/x86_64/bad-reg.rs

@@ -21,10 +21,6 @@ fn main() {
         //~^ ERROR asm template modifiers are not allowed for `const` arguments
         asm!("{:a}", sym main);
         //~^ ERROR asm template modifiers are not allowed for `sym` arguments
-        asm!("{}", in(zmm_reg) foo);
-        //~^ ERROR register class `zmm_reg` requires the `avx512f` target feature
-        asm!("", in("zmm0") foo);
-        //~^ ERROR register class `zmm_reg` requires the `avx512f` target feature
         asm!("", in("ebp") foo);
         //~^ ERROR invalid register `ebp`: the frame pointer cannot be used as an operand
         asm!("", in("rsp") foo);

src/test/ui/asm/x86_64/bad-reg.stderr

@@ -46,86 +46,74 @@ LL |         asm!("{:a}", sym main);
    |               |
    |               template modifier
 
-error: register class `zmm_reg` requires the `avx512f` target feature
-  --> $DIR/bad-reg.rs:24:20
-   |
-LL |         asm!("{}", in(zmm_reg) foo);
-   |                    ^^^^^^^^^^^^^^^
-
-error: register class `zmm_reg` requires the `avx512f` target feature
-  --> $DIR/bad-reg.rs:26:18
-   |
-LL |         asm!("", in("zmm0") foo);
-   |                  ^^^^^^^^^^^^^^
-
 error: invalid register `ebp`: the frame pointer cannot be used as an operand for inline asm
-  --> $DIR/bad-reg.rs:28:18
+  --> $DIR/bad-reg.rs:24:18
    |
 LL |         asm!("", in("ebp") foo);
    |                  ^^^^^^^^^^^^^
 
 error: invalid register `rsp`: the stack pointer cannot be used as an operand for inline asm
-  --> $DIR/bad-reg.rs:30:18
+  --> $DIR/bad-reg.rs:26:18
    |
 LL |         asm!("", in("rsp") foo);
    |                  ^^^^^^^^^^^^^
 
 error: invalid register `ip`: the instruction pointer cannot be used as an operand for inline asm
-  --> $DIR/bad-reg.rs:32:18
+  --> $DIR/bad-reg.rs:28:18
    |
 LL |         asm!("", in("ip") foo);
    |                  ^^^^^^^^^^^^
 
 error: invalid register `k0`: the k0 AVX mask register cannot be used as an operand for inline asm
-  --> $DIR/bad-reg.rs:34:18
+  --> $DIR/bad-reg.rs:30:18
    |
 LL |         asm!("", in("k0") foo);
    |                  ^^^^^^^^^^^^
 
 error: invalid register `ah`: high byte registers cannot be used as an operand on x86_64
-  --> $DIR/bad-reg.rs:36:18
+  --> $DIR/bad-reg.rs:32:18
    |
 LL |         asm!("", in("ah") foo);
    |                  ^^^^^^^^^^^^
 
 error: register class `x87_reg` can only be used as a clobber, not as an input or output
-  --> $DIR/bad-reg.rs:39:18
+  --> $DIR/bad-reg.rs:35:18
    |
 LL |         asm!("", in("st(2)") foo);
    |                  ^^^^^^^^^^^^^^^
 
 error: register class `mmx_reg` can only be used as a clobber, not as an input or output
-  --> $DIR/bad-reg.rs:41:18
+  --> $DIR/bad-reg.rs:37:18
    |
 LL |         asm!("", in("mm0") foo);
    |                  ^^^^^^^^^^^^^
 
 error: register class `x87_reg` can only be used as a clobber, not as an input or output
-  --> $DIR/bad-reg.rs:45:20
+  --> $DIR/bad-reg.rs:41:20
    |
 LL |         asm!("{}", in(x87_reg) foo);
    |                    ^^^^^^^^^^^^^^^
 
 error: register class `mmx_reg` can only be used as a clobber, not as an input or output
-  --> $DIR/bad-reg.rs:47:20
+  --> $DIR/bad-reg.rs:43:20
    |
 LL |         asm!("{}", in(mmx_reg) foo);
    |                    ^^^^^^^^^^^^^^^
 
 error: register class `x87_reg` can only be used as a clobber, not as an input or output
-  --> $DIR/bad-reg.rs:49:20
+  --> $DIR/bad-reg.rs:45:20
    |
 LL |         asm!("{}", out(x87_reg) _);
    |                    ^^^^^^^^^^^^^^
 
 error: register class `mmx_reg` can only be used as a clobber, not as an input or output
-  --> $DIR/bad-reg.rs:51:20
+  --> $DIR/bad-reg.rs:47:20
    |
 LL |         asm!("{}", out(mmx_reg) _);
    |                    ^^^^^^^^^^^^^^
 
 error: register `al` conflicts with register `ax`
-  --> $DIR/bad-reg.rs:57:33
+  --> $DIR/bad-reg.rs:53:33
    |
 LL |         asm!("", in("eax") foo, in("al") bar);
    |                  -------------  ^^^^^^^^^^^^ register `al`
@@ -133,7 +121,7 @@ LL |         asm!("", in("eax") foo, in("al") bar);
    |                  register `ax`
 
 error: register `ax` conflicts with register `ax`
-  --> $DIR/bad-reg.rs:59:33
+  --> $DIR/bad-reg.rs:55:33
    |
 LL |         asm!("", in("rax") foo, out("rax") bar);
    |                  -------------  ^^^^^^^^^^^^^^ register `ax`
@@ -141,13 +129,13 @@ LL |         asm!("", in("rax") foo, out("rax") bar);
    |                  register `ax`
    |
 help: use `lateout` instead of `out` to avoid conflict
-  --> $DIR/bad-reg.rs:59:18
+  --> $DIR/bad-reg.rs:55:18
    |
 LL |         asm!("", in("rax") foo, out("rax") bar);
    |                  ^^^^^^^^^^^^^
 
 error: register `ymm0` conflicts with register `xmm0`
-  --> $DIR/bad-reg.rs:62:34
+  --> $DIR/bad-reg.rs:58:34
    |
 LL |         asm!("", in("xmm0") foo, in("ymm0") bar);
    |                  --------------  ^^^^^^^^^^^^^^ register `ymm0`
@@ -155,7 +143,7 @@ LL |         asm!("", in("xmm0") foo, in("ymm0") bar);
    |                  register `xmm0`
 
 error: register `ymm0` conflicts with register `xmm0`
-  --> $DIR/bad-reg.rs:64:34
+  --> $DIR/bad-reg.rs:60:34
    |
 LL |         asm!("", in("xmm0") foo, out("ymm0") bar);
    |                  --------------  ^^^^^^^^^^^^^^^ register `ymm0`
@@ -163,10 +151,10 @@ LL |         asm!("", in("xmm0") foo, out("ymm0") bar);
    |                  register `xmm0`
    |
 help: use `lateout` instead of `out` to avoid conflict
-  --> $DIR/bad-reg.rs:64:18
+  --> $DIR/bad-reg.rs:60:18
    |
 LL |         asm!("", in("xmm0") foo, out("ymm0") bar);
    |                  ^^^^^^^^^^^^^^
 
-error: aborting due to 23 previous errors
+error: aborting due to 21 previous errors
 

src/test/ui/asm/x86_64/target-feature-attr.rs

@@ -0,0 +1,40 @@
+// only-x86_64
+
+#![feature(asm, avx512_target_feature)]
+
+#[target_feature(enable = "avx")]
+unsafe fn foo() {
+    let mut x = 1;
+    let y = 2;
+    asm!("vaddps {2:y}, {0:y}, {1:y}", in(ymm_reg) x, in(ymm_reg) y, lateout(ymm_reg) x);
+    assert_eq!(x, 3);
+}
+
+unsafe fn bar() {
+    let mut x = 1;
+    let y = 2;
+    asm!("vaddps {2:y}, {0:y}, {1:y}", in(ymm_reg) x, in(ymm_reg) y, lateout(ymm_reg) x);
+    //~^ ERROR: register class `ymm_reg` requires the `avx` target feature
+    //~| ERROR: register class `ymm_reg` requires the `avx` target feature
+    //~| ERROR: register class `ymm_reg` requires the `avx` target feature
+    assert_eq!(x, 3);
+}
+
+#[target_feature(enable = "avx512bw")]
+unsafe fn baz() {
+    let x = 1;
+    asm!("/* {0} */", in(kreg) x);
+}
+
+unsafe fn baz2() {
+    let x = 1;
+    asm!("/* {0} */", in(kreg) x);
+    //~^ ERROR: register class `kreg` requires at least one of the following target features: avx512bw, avx512f
+}
+
+fn main() {
+    unsafe {
+        foo();
+        bar();
+    }
+}

src/test/ui/asm/x86_64/target-feature-attr.stderr

@@ -0,0 +1,26 @@
+error: register class `ymm_reg` requires the `avx` target feature
+  --> $DIR/target-feature-attr.rs:16:40
+   |
+LL |     asm!("vaddps {2:y}, {0:y}, {1:y}", in(ymm_reg) x, in(ymm_reg) y, lateout(ymm_reg) x);
+   |                                        ^^^^^^^^^^^^^
+
+error: register class `ymm_reg` requires the `avx` target feature
+  --> $DIR/target-feature-attr.rs:16:55
+   |
+LL |     asm!("vaddps {2:y}, {0:y}, {1:y}", in(ymm_reg) x, in(ymm_reg) y, lateout(ymm_reg) x);
+   |                                                       ^^^^^^^^^^^^^
+
+error: register class `ymm_reg` requires the `avx` target feature
+  --> $DIR/target-feature-attr.rs:16:70
+   |
+LL |     asm!("vaddps {2:y}, {0:y}, {1:y}", in(ymm_reg) x, in(ymm_reg) y, lateout(ymm_reg) x);
+   |                                                                      ^^^^^^^^^^^^^^^^^^
+
+error: register class `kreg` requires at least one of the following target features: avx512bw, avx512f
+  --> $DIR/target-feature-attr.rs:31:23
+   |
+LL |     asm!("/* {0} */", in(kreg) x);
+   |                       ^^^^^^^^^^
+
+error: aborting due to 4 previous errors
+