Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit ff97db1

Browse files
AmanieuAmanieu
committed
Apply review feedback
1 parent 8ab0f2d commit ff97db1

File tree

10 files changed

+185
-139
lines changed

10 files changed

+185
-139
lines changed

‎src/librustc_builtin_macros/asm.rs

Lines changed: 66 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -80,47 +80,14 @@ fn parse_args<'a>(
8080
break;
8181
} // accept trailing commas
8282

83-
let span_start = p.token.span;
84-
8583
// Parse options
8684
if p.eat(&token::Ident(sym::options, false)) {
87-
p.expect(&token::OpenDelim(token::DelimToken::Paren))?;
88-
89-
while !p.eat(&token::CloseDelim(token::DelimToken::Paren)) {
90-
if p.eat(&token::Ident(sym::pure, false)) {
91-
args.options |= InlineAsmOptions::PURE;
92-
} else if p.eat(&token::Ident(sym::nomem, false)) {
93-
args.options |= InlineAsmOptions::NOMEM;
94-
} else if p.eat(&token::Ident(sym::readonly, false)) {
95-
args.options |= InlineAsmOptions::READONLY;
96-
} else if p.eat(&token::Ident(sym::preserves_flags, false)) {
97-
args.options |= InlineAsmOptions::PRESERVES_FLAGS;
98-
} else if p.eat(&token::Ident(sym::noreturn, false)) {
99-
args.options |= InlineAsmOptions::NORETURN;
100-
} else {
101-
p.expect(&token::Ident(sym::nostack, false))?;
102-
args.options |= InlineAsmOptions::NOSTACK;
103-
}
104-
105-
// Allow trailing commas
106-
if p.eat(&token::CloseDelim(token::DelimToken::Paren)) {
107-
break;
108-
}
109-
p.expect(&token::Comma)?;
110-
}
111-
112-
let new_span = span_start.to(p.prev_token.span);
113-
if let Some(options_span) = args.options_span {
114-
ecx.struct_span_err(new_span, "asm options cannot be specified twice")
115-
.span_label(options_span, "previously here")
116-
.span_label(new_span, "duplicate options")
117-
.emit();
118-
} else {
119-
args.options_span = Some(new_span);
120-
}
85+
parse_options(&mut p, &mut args)?;
12186
continue;
12287
}
12388

89+
let span_start = p.token.span;
90+
12491
// Parse operand names
12592
let name = if p.token.is_ident() && p.look_ahead(1, |t| *t == token::Eq) {
12693
let (ident, _) = p.token.ident().unwrap();
@@ -131,29 +98,6 @@ fn parse_args<'a>(
13198
None
13299
};
133100

134-
fn parse_reg<'a>(
135-
p: &mut Parser<'a>,
136-
explicit_reg: &mut bool,
137-
) -> Result<ast::InlineAsmRegOrRegClass, DiagnosticBuilder<'a>> {
138-
p.expect(&token::OpenDelim(token::DelimToken::Paren))?;
139-
let result = match p.token.kind {
140-
token::Ident(name, false) => ast::InlineAsmRegOrRegClass::RegClass(name),
141-
token::Literal(token::Lit { kind: token::LitKind::Str, symbol, suffix: _ }) => {
142-
*explicit_reg = true;
143-
ast::InlineAsmRegOrRegClass::Reg(symbol)
144-
}
145-
_ => {
146-
return Err(p.struct_span_err(
147-
p.token.span,
148-
"expected register class or explicit register",
149-
));
150-
}
151-
};
152-
p.bump();
153-
p.expect(&token::CloseDelim(token::DelimToken::Paren))?;
154-
Ok(result)
155-
};
156-
157101
let mut explicit_reg = false;
158102
let op = if p.eat(&token::Ident(kw::In, false)) {
159103
let reg = parse_reg(&mut p, &mut explicit_reg)?;
@@ -319,6 +263,69 @@ fn parse_args<'a>(
319263
Ok(args)
320264
}
321265

266+
fn parse_options<'a>(p: &mut Parser<'a>, args: &mut AsmArgs) -> Result<(), DiagnosticBuilder<'a>> {
267+
let span_start = p.prev_token.span;
268+
269+
p.expect(&token::OpenDelim(token::DelimToken::Paren))?;
270+
271+
while !p.eat(&token::CloseDelim(token::DelimToken::Paren)) {
272+
if p.eat(&token::Ident(sym::pure, false)) {
273+
args.options |= InlineAsmOptions::PURE;
274+
} else if p.eat(&token::Ident(sym::nomem, false)) {
275+
args.options |= InlineAsmOptions::NOMEM;
276+
} else if p.eat(&token::Ident(sym::readonly, false)) {
277+
args.options |= InlineAsmOptions::READONLY;
278+
} else if p.eat(&token::Ident(sym::preserves_flags, false)) {
279+
args.options |= InlineAsmOptions::PRESERVES_FLAGS;
280+
} else if p.eat(&token::Ident(sym::noreturn, false)) {
281+
args.options |= InlineAsmOptions::NORETURN;
282+
} else {
283+
p.expect(&token::Ident(sym::nostack, false))?;
284+
args.options |= InlineAsmOptions::NOSTACK;
285+
}
286+
287+
// Allow trailing commas
288+
if p.eat(&token::CloseDelim(token::DelimToken::Paren)) {
289+
break;
290+
}
291+
p.expect(&token::Comma)?;
292+
}
293+
294+
let new_span = span_start.to(p.prev_token.span);
295+
if let Some(options_span) = args.options_span {
296+
p.struct_span_err(new_span, "asm options cannot be specified multiple times")
297+
.span_label(options_span, "previously here")
298+
.span_label(new_span, "duplicate options")
299+
.emit();
300+
} else {
301+
args.options_span = Some(new_span);
302+
}
303+
304+
Ok(())
305+
}
306+
307+
fn parse_reg<'a>(
308+
p: &mut Parser<'a>,
309+
explicit_reg: &mut bool,
310+
) -> Result<ast::InlineAsmRegOrRegClass, DiagnosticBuilder<'a>> {
311+
p.expect(&token::OpenDelim(token::DelimToken::Paren))?;
312+
let result = match p.token.kind {
313+
token::Ident(name, false) => ast::InlineAsmRegOrRegClass::RegClass(name),
314+
token::Literal(token::Lit { kind: token::LitKind::Str, symbol, suffix: _ }) => {
315+
*explicit_reg = true;
316+
ast::InlineAsmRegOrRegClass::Reg(symbol)
317+
}
318+
_ => {
319+
return Err(
320+
p.struct_span_err(p.token.span, "expected register class or explicit register")
321+
);
322+
}
323+
};
324+
p.bump();
325+
p.expect(&token::CloseDelim(token::DelimToken::Paren))?;
326+
Ok(result)
327+
}
328+
322329
fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P<ast::Expr> {
323330
let msg = "asm template must be a string literal";
324331
let template_sp = args.template.span;

‎src/librustc_passes/intrinsicck.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,10 @@ impl ExprVisitor<'tcx> {
239239
&format!("type `{}`", self.tables.expr_ty_adjusted(in_expr)),
240240
);
241241
err.span_label(expr.span, &format!("type `{}`", ty));
242-
err.note("asm inout arguments must have the same type");
243-
err.note("unless they are both pointers or integers of the same size");
242+
err.note(
243+
"asm inout arguments must have the same type, \
244+
unless they are both pointers or integers of the same size",
245+
);
244246
err.emit();
245247
}
246248

@@ -271,7 +273,16 @@ impl ExprVisitor<'tcx> {
271273
}
272274
};
273275

274-
// Check whether the selected type requires a target feature.
276+
// Check whether the selected type requires a target feature. Note that
277+
// this is different from the feature check we did earlier in AST
278+
// lowering. While AST lowering checked that this register class is
279+
// usable at all with the currently enabled features, some types may
280+
// only be usable with a register class when a certain feature is
281+
// enabled. We check this here since it depends on the results of typeck.
282+
//
283+
// Also note that this check isn't run when the operand type is never
284+
// (!). In that case we still need the earlier check in AST lowering to
285+
// verify that the register class is usable at all.
275286
if let Some(feature) = feature {
276287
if !self.tcx.sess.target_features.contains(&Symbol::intern(feature)) {
277288
let msg = &format!("`{}` target feature is not enabled", feature);

‎src/librustc_target/asm/aarch64.rs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,10 @@ impl AArch64InlineAsmRegClass {
2424
ty: InlineAsmType,
2525
) -> Option<(char, &'static str, Option<&'static str>)> {
2626
match self {
27-
Self::reg => {
28-
if ty.size().bits() <= 32 {
29-
Some(('w', "w0", None))
30-
} else {
31-
None
32-
}
33-
}
27+
Self::reg => match ty.size().bits() {
28+
64 => None,
29+
_ => Some(('w', "w0", None)),
30+
},
3431
Self::vreg | Self::vreg_low16 => match ty.size().bits() {
3532
8 => Some(('b', "b0", None)),
3633
16 => Some(('h', "h0", None)),
@@ -57,8 +54,8 @@ impl AArch64InlineAsmRegClass {
5754
Self::reg => types! { _: I8, I16, I32, I64, F32, F64; },
5855
Self::vreg | Self::vreg_low16 => types! {
5956
"fp": I8, I16, I32, I64, F32, F64,
60-
VecI8(8), VecI16(4), VecI32(2), VecI64(1), VecF32(2), VecF64(1),
61-
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2);
57+
VecI8(8), VecI16(4), VecI32(2), VecI64(1), VecF32(2), VecF64(1),
58+
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2);
6259
},
6360
}
6461
}
@@ -128,12 +125,12 @@ def_regs! {
128125
v29: vreg = ["v29", "b29", "h29", "s29", "d29", "q29"],
129126
v30: vreg = ["v30", "b30", "h30", "s30", "d30", "q30"],
130127
v31: vreg = ["v31", "b31", "h31", "s31", "d31", "q31"],
131-
"the frame pointer cannot be used as an operand for inline asm" =
132-
["x29", "fp"],
133-
"the stack pointer cannot be used as an operand for inline asm" =
134-
["sp", "wsp"],
135-
"the zero register cannot be used as an operand for inline asm" =
136-
["xzr", "wzr"],
128+
#error = ["x29", "fp"] =>
129+
"the frame pointer cannot be used as an operand for inline asm",
130+
#error = ["sp", "wsp"] =>
131+
"the stack pointer cannot be used as an operand for inline asm",
132+
#error = ["xzr", "wzr"] =>
133+
"the zero register cannot be used as an operand for inline asm",
137134
}
138135
}
139136

‎src/librustc_target/asm/arm.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,12 @@ def_regs! {
149149
q13: qreg = ["q13"],
150150
q14: qreg = ["q14"],
151151
q15: qreg = ["q15"],
152-
"the frame pointer cannot be used as an operand for inline asm" =
153-
["r11", "fp"],
154-
"the stack pointer cannot be used as an operand for inline asm" =
155-
["r13", "sp"],
156-
"the program pointer cannot be used as an operand for inline asm" =
157-
["r15", "pc"],
152+
#error = ["r11", "fp"] =>
153+
"the frame pointer cannot be used as an operand for inline asm",
154+
#error = ["r13", "sp"] =>
155+
"the stack pointer cannot be used as an operand for inline asm",
156+
#error = ["r15", "pc"] =>
157+
"the program pointer cannot be used as an operand for inline asm",
158158
}
159159
}
160160

@@ -231,6 +231,15 @@ impl ArmInlineAsmReg {
231231
}
232232
};
233233
}
234+
235+
// ARM's floating-point register file is interesting in that it can be
236+
// viewed as 16 128-bit registers, 32 64-bit registers or 32 32-bit
237+
// registers. Because these views overlap, the registers of different
238+
// widths will conflict (e.g. d0 overlaps with s0 and s1, and q1
239+
// overlaps with d2 and d3).
240+
//
241+
// See section E1.3.1 of the ARM Architecture Reference Manual for
242+
// ARMv8-A for more details.
234243
reg_conflicts! {
235244
q0 : d0 d1 : s0 s1 s2 s3,
236245
q1 : d2 d3 : s4 s5 s6 s7,

‎src/librustc_target/asm/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ macro_rules! def_regs {
5757
$reg:ident: $class:ident $(, $extra_class:ident)* = [$reg_name:literal $(, $alias:literal)*] $(% $filter:ident)?,
5858
)*
5959
$(
60-
$error:literal = [$($bad_reg:literal),+],
60+
#error = [$($bad_reg:literal),+] => $error:literal,
6161
)*
6262
}) => {
6363
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, Eq, PartialEq, Hash, HashStable_Generic)]

‎src/librustc_target/asm/riscv.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -115,16 +115,16 @@ def_regs! {
115115
f29: freg = ["f29", "ft9"],
116116
f30: freg = ["f30", "ft10"],
117117
f31: freg = ["f31", "ft11"],
118-
"the frame pointer cannot be used as an operand for inline asm" =
119-
["x8", "s0", "fp"],
120-
"the stack pointer cannot be used as an operand for inline asm" =
121-
["x2", "sp"],
122-
"the global pointer cannot be used as an operand for inline asm" =
123-
["x3", "gp"],
124-
"the thread pointer cannot be used as an operand for inline asm" =
125-
["x4", "tp"],
126-
"the zero register cannot be used as an operand for inline asm" =
127-
["x0", "zero"],
118+
#error = ["x8", "s0", "fp"] =>
119+
"the frame pointer cannot be used as an operand for inline asm",
120+
#error = ["x2", "sp"] =>
121+
"the stack pointer cannot be used as an operand for inline asm",
122+
#error = ["x3", "gp"] =>
123+
"the global pointer cannot be used as an operand for inline asm",
124+
#error = ["x4", "tp"] =>
125+
"the thread pointer cannot be used as an operand for inline asm" ,
126+
#error = ["x0", "zero"] =>
127+
"the zero register cannot be used as an operand for inline asm",
128128
}
129129
}
130130

‎src/librustc_target/asm/x86.rs

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,13 @@ impl X86InlineAsmRegClass {
6262
_ => None,
6363
},
6464
Self::xmm_reg => None,
65-
Self::ymm_reg => {
66-
if ty.size().bits() <= 128 {
67-
Some(('x', "xmm0", None))
68-
} else {
69-
None
70-
}
71-
}
65+
Self::ymm_reg => match ty.size().bits() {
66+
256 => None,
67+
_ => Some(('x', "xmm0", None)),
68+
},
7269
Self::zmm_reg => match ty.size().bits() {
73-
256 => Some(('y', "ymm0", None)),
7470
512 => None,
71+
256 => Some(('y', "ymm0", None)),
7572
_ => Some(('x', "xmm0", None)),
7673
},
7774
Self::kreg => None,
@@ -108,18 +105,18 @@ impl X86InlineAsmRegClass {
108105
}
109106
Self::xmm_reg => types! {
110107
"sse": I32, I64, F32, F64,
111-
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2);
108+
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2);
112109
},
113110
Self::ymm_reg => types! {
114111
"avx": I32, I64, F32, F64,
115-
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2),
116-
VecI8(32), VecI16(16), VecI32(8), VecI64(4), VecF32(8), VecF64(4);
112+
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2),
113+
VecI8(32), VecI16(16), VecI32(8), VecI64(4), VecF32(8), VecF64(4);
117114
},
118115
Self::zmm_reg => types! {
119116
"avx512f": I32, I64, F32, F64,
120-
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2),
121-
VecI8(32), VecI16(16), VecI32(8), VecI64(4), VecF32(8), VecF64(4),
122-
VecI8(64), VecI16(32), VecI32(16), VecI64(8), VecF32(16), VecF64(8);
117+
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2),
118+
VecI8(32), VecI16(16), VecI32(8), VecI64(4), VecF32(8), VecF64(4),
119+
VecI8(64), VecI16(32), VecI32(16), VecI64(8), VecF32(16), VecF64(8);
123120
},
124121
Self::kreg => types! {
125122
"avx512f": I8, I16;
@@ -227,19 +224,20 @@ def_regs! {
227224
k5: kreg = ["k5"],
228225
k6: kreg = ["k6"],
229226
k7: kreg = ["k7"],
230-
"high byte registers are not currently supported as operands for inline asm" =
231-
["ah", "bh", "ch", "dh"],
232-
"the frame pointer cannot be used as an operand for inline asm" =
233-
["bp", "bpl", "ebp", "rbp"],
234-
"the stack pointer cannot be used as an operand for inline asm" =
235-
["sp", "spl", "esp", "rsp"],
236-
"the instruction pointer cannot be used as an operand for inline asm" =
237-
["ip", "eip", "rip"],
238-
"x87 registers are not currently supported as operands for inline asm" =
239-
["st", "st(0)", "st(1)", "st(2)", "st(3)", "st(4)", "st(5)", "st(6)", "st(7)"],
240-
"MMX registers are not currently supported as operands for inline asm" =
241-
["mm0", "mm1", "mm2", "mm3", "mm4", "mm5", "mm6", "mm7"],
242-
"the k0 AVX mask register cannot be used as an operand for inline asm" = ["k0"],
227+
#error = ["ah", "bh", "ch", "dh"] =>
228+
"high byte registers are not currently supported as operands for inline asm",
229+
#error = ["bp", "bpl", "ebp", "rbp"] =>
230+
"the frame pointer cannot be used as an operand for inline asm",
231+
#error = ["sp", "spl", "esp", "rsp"] =>
232+
"the stack pointer cannot be used as an operand for inline asm",
233+
#error = ["ip", "eip", "rip"] =>
234+
"the instruction pointer cannot be used as an operand for inline asm",
235+
#error = ["st", "st(0)", "st(1)", "st(2)", "st(3)", "st(4)", "st(5)", "st(6)", "st(7)"] =>
236+
"x87 registers are not currently supported as operands for inline asm",
237+
#error = ["mm0", "mm1", "mm2", "mm3", "mm4", "mm5", "mm6", "mm7"] =>
238+
"MMX registers are not currently supported as operands for inline asm",
239+
#error = ["k0"] =>
240+
"the k0 AVX mask register cannot be used as an operand for inline asm",
243241
}
244242
}
245243

@@ -316,6 +314,15 @@ impl X86InlineAsmReg {
316314
}
317315
};
318316
}
317+
318+
// XMM*, YMM* and ZMM* are all different views of the same register.
319+
//
320+
// See section 15.5 of the combined Intel® 64 and IA-32 Architectures
321+
// Software Developer’s Manual for more details.
322+
//
323+
// We don't need to specify conflicts for [x,y,z]mm[16-31] since these
324+
// registers are only available with AVX-512, so we just specify them
325+
// as aliases directly.
319326
reg_conflicts! {
320327
xmm0 : ymm0 : zmm0,
321328
xmm1 : ymm1 : zmm1,

0 commit comments

Comments
 (0)