Skip to content

Commit ddb015a

Browse files
authored
Fix discarded in-out constraint in inline asm (rust-lang#110)
Fixes rust-lang#109
1 parent ebe6f67 commit ddb015a

File tree

2 files changed

+73
-65
lines changed

2 files changed

+73
-65
lines changed

src/asm.rs

Lines changed: 51 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -18,30 +18,30 @@ use crate::type_of::LayoutGccExt;
1818

1919
// Rust asm! and GCC Extended Asm semantics differ substantially.
2020
//
21-
// 1. Rust asm operands go along as one list of operands. Operands themselves indicate
22-
// if they're "in" or "out". "In" and "out" operands can interleave. One operand can be
21+
// 1. Rust asm operands go along as one list of operands. Operands themselves indicate
22+
// if they're "in" or "out". "In" and "out" operands can interleave. One operand can be
2323
// both "in" and "out" (`inout(reg)`).
2424
//
25-
// GCC asm has two different lists for "in" and "out" operands. In terms of gccjit,
26-
// this means that all "out" operands must go before "in" operands. "In" and "out" operands
25+
// GCC asm has two different lists for "in" and "out" operands. In terms of gccjit,
26+
// this means that all "out" operands must go before "in" operands. "In" and "out" operands
2727
// cannot interleave.
2828
//
29-
// 2. Operand lists in both Rust and GCC are indexed. Index starts from 0. Indexes are important
29+
// 2. Operand lists in both Rust and GCC are indexed. Index starts from 0. Indexes are important
3030
// because the asm template refers to operands by index.
3131
//
3232
// Mapping from Rust to GCC index would be 1-1 if it wasn't for...
3333
//
34-
// 3. Clobbers. GCC has a separate list of clobbers, and clobbers don't have indexes.
35-
// Contrary, Rust expresses clobbers through "out" operands that aren't tied to
34+
// 3. Clobbers. GCC has a separate list of clobbers, and clobbers don't have indexes.
35+
// Contrary, Rust expresses clobbers through "out" operands that aren't tied to
3636
// a variable (`_`), and such "clobbers" do have index.
3737
//
38-
// 4. Furthermore, GCC Extended Asm does not support explicit register constraints
39-
// (like `out("eax")`) directly, offering so-called "local register variables"
40-
// as a workaround. These variables need to be declared and initialized *before*
41-
// the Extended Asm block but *after* normal local variables
38+
// 4. Furthermore, GCC Extended Asm does not support explicit register constraints
39+
// (like `out("eax")`) directly, offering so-called "local register variables"
40+
// as a workaround. These variables need to be declared and initialized *before*
41+
// the Extended Asm block but *after* normal local variables
4242
// (see comment in `codegen_inline_asm` for explanation).
4343
//
44-
// With that in mind, let's see how we translate Rust syntax to GCC
44+
// With that in mind, let's see how we translate Rust syntax to GCC
4545
// (from now on, `CC` stands for "constraint code"):
4646
//
4747
// * `out(reg_class) var` -> translated to output operand: `"=CC"(var)`
@@ -52,18 +52,17 @@ use crate::type_of::LayoutGccExt;
5252
//
5353
// * `out("explicit register") _` -> not translated to any operands, register is simply added to clobbers list
5454
//
55-
// * `inout(reg_class) in_var => out_var` -> translated to two operands:
55+
// * `inout(reg_class) in_var => out_var` -> translated to two operands:
5656
// output: `"=CC"(in_var)`
57-
// input: `"num"(out_var)` where num is the GCC index
57+
// input: `"num"(out_var)` where num is the GCC index
5858
// of the corresponding output operand
5959
//
60-
// * `inout(reg_class) in_var => _` -> same as `inout(reg_class) in_var => tmp`,
60+
// * `inout(reg_class) in_var => _` -> same as `inout(reg_class) in_var => tmp`,
6161
// where "tmp" is a temporary unused variable
6262
//
63-
// * `out/in/inout("explicit register") var` -> translated to one or two operands as described above
64-
// with `"r"(var)` constraint,
63+
// * `out/in/inout("explicit register") var` -> translated to one or two operands as described above
64+
// with `"r"(var)` constraint,
6565
// and one register variable assigned to the desired register.
66-
//
6766

6867
const ATT_SYNTAX_INS: &str = ".att_syntax noprefix\n\t";
6968
const INTEL_SYNTAX_INS: &str = "\n\t.intel_syntax noprefix";
@@ -124,7 +123,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
124123
let att_dialect = is_x86 && options.contains(InlineAsmOptions::ATT_SYNTAX);
125124
let intel_dialect = is_x86 && !options.contains(InlineAsmOptions::ATT_SYNTAX);
126125

127-
// GCC index of an output operand equals its position in the array
126+
// GCC index of an output operand equals its position in the array
128127
let mut outputs = vec![];
129128

130129
// GCC index of an input operand equals its position in the array
@@ -138,9 +137,9 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
138137
let mut constants_len = 0;
139138

140139
// There are rules we must adhere to if we want GCC to do the right thing:
141-
//
140+
//
142141
// * Every local variable that the asm block uses as an output must be declared *before*
143-
// the asm block.
142+
// the asm block.
144143
// * There must be no instructions whatsoever between the register variables and the asm.
145144
//
146145
// Therefore, the backend must generate the instructions strictly in this order:
@@ -152,7 +151,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
152151
// We also must make sure that no input operands are emitted before output operands.
153152
//
154153
// This is why we work in passes, first emitting local vars, then local register vars.
155-
// Also, we don't emit any asm operands immediately; we save them to
154+
// Also, we don't emit any asm operands immediately; we save them to
156155
// the one of the buffers to be emitted later.
157156

158157
// 1. Normal variables (and saving operands to buffers).
@@ -165,7 +164,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
165164
(Constraint(constraint), Some(place)) => (constraint, place.layout.gcc_type(self.cx, false)),
166165
// When `reg` is a class and not an explicit register but the out place is not specified,
167166
// we need to create an unused output variable to assign the output to. This var
168-
// needs to be of a type that's "compatible" with the register class, but specific type
167+
// needs to be of a type that's "compatible" with the register class, but specific type
169168
// doesn't matter.
170169
(Constraint(constraint), None) => (constraint, dummy_output_type(self.cx, reg.reg_class())),
171170
(Register(_), Some(_)) => {
@@ -193,7 +192,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
193192

194193
let tmp_var = self.current_func().new_local(None, ty, "output_register");
195194
outputs.push(AsmOutOperand {
196-
constraint,
195+
constraint,
197196
rust_idx,
198197
late,
199198
readwrite: false,
@@ -204,12 +203,12 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
204203

205204
InlineAsmOperandRef::In { reg, value } => {
206205
if let ConstraintOrRegister::Constraint(constraint) = reg_to_gcc(reg) {
207-
inputs.push(AsmInOperand {
208-
constraint: Cow::Borrowed(constraint),
209-
rust_idx,
206+
inputs.push(AsmInOperand {
207+
constraint: Cow::Borrowed(constraint),
208+
rust_idx,
210209
val: value.immediate()
211210
});
212-
}
211+
}
213212
else {
214213
// left for the next pass
215214
continue
@@ -219,7 +218,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
219218
InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => {
220219
let constraint = if let ConstraintOrRegister::Constraint(constraint) = reg_to_gcc(reg) {
221220
constraint
222-
}
221+
}
223222
else {
224223
// left for the next pass
225224
continue
@@ -228,22 +227,22 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
228227
// Rustc frontend guarantees that input and output types are "compatible",
229228
// so we can just use input var's type for the output variable.
230229
//
231-
// This decision is also backed by the fact that LLVM needs in and out
232-
// values to be of *exactly the same type*, not just "compatible".
230+
// This decision is also backed by the fact that LLVM needs in and out
231+
// values to be of *exactly the same type*, not just "compatible".
233232
// I'm not sure if GCC is so picky too, but better safe than sorry.
234233
let ty = in_value.layout.gcc_type(self.cx, false);
235234
let tmp_var = self.current_func().new_local(None, ty, "output_register");
236235

237236
// If the out_place is None (i.e `inout(reg) _` syntax was used), we translate
238-
// it to one "readwrite (+) output variable", otherwise we translate it to two
237+
// it to one "readwrite (+) output variable", otherwise we translate it to two
239238
// "out and tied in" vars as described above.
240239
let readwrite = out_place.is_none();
241240
outputs.push(AsmOutOperand {
242-
constraint,
241+
constraint,
243242
rust_idx,
244243
late,
245244
readwrite,
246-
tmp_var,
245+
tmp_var,
247246
out_place,
248247
});
249248

@@ -252,8 +251,8 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
252251
let constraint = Cow::Owned(out_gcc_idx.to_string());
253252

254253
inputs.push(AsmInOperand {
255-
constraint,
256-
rust_idx,
254+
constraint,
255+
rust_idx,
257256
val: in_value.immediate()
258257
});
259258
}
@@ -280,7 +279,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
280279
if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) {
281280
let out_place = if let Some(place) = place {
282281
place
283-
}
282+
}
284283
else {
285284
// processed in the previous pass
286285
continue
@@ -291,7 +290,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
291290
tmp_var.set_register_name(reg_name);
292291

293292
outputs.push(AsmOutOperand {
294-
constraint: "r".into(),
293+
constraint: "r".into(),
295294
rust_idx,
296295
late,
297296
readwrite: false,
@@ -311,9 +310,9 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
311310
reg_var.set_register_name(reg_name);
312311
self.llbb().add_assignment(None, reg_var, value.immediate());
313312

314-
inputs.push(AsmInOperand {
315-
constraint: "r".into(),
316-
rust_idx,
313+
inputs.push(AsmInOperand {
314+
constraint: "r".into(),
315+
rust_idx,
317316
val: reg_var.to_rvalue()
318317
});
319318
}
@@ -324,31 +323,23 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
324323
// `inout("explicit register") in_var => out_var`
325324
InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => {
326325
if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) {
327-
let out_place = if let Some(place) = out_place {
328-
place
329-
}
330-
else {
331-
// processed in the previous pass
332-
continue
333-
};
334-
335326
// See explanation in the first pass.
336327
let ty = in_value.layout.gcc_type(self.cx, false);
337328
let tmp_var = self.current_func().new_local(None, ty, "output_register");
338329
tmp_var.set_register_name(reg_name);
339330

340331
outputs.push(AsmOutOperand {
341-
constraint: "r".into(),
332+
constraint: "r".into(),
342333
rust_idx,
343334
late,
344335
readwrite: false,
345336
tmp_var,
346-
out_place: Some(out_place)
337+
out_place,
347338
});
348339

349340
let constraint = Cow::Owned((outputs.len() - 1).to_string());
350-
inputs.push(AsmInOperand {
351-
constraint,
341+
inputs.push(AsmInOperand {
342+
constraint,
352343
rust_idx,
353344
val: in_value.immediate()
354345
});
@@ -357,8 +348,8 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
357348
// processed in the previous pass
358349
}
359350

360-
InlineAsmOperandRef::Const { .. }
361-
| InlineAsmOperandRef::SymFn { .. }
351+
InlineAsmOperandRef::Const { .. }
352+
| InlineAsmOperandRef::SymFn { .. }
362353
| InlineAsmOperandRef::SymStatic { .. } => {
363354
// processed in the previous pass
364355
}
@@ -453,7 +444,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
453444
if !intel_dialect {
454445
template_str.push_str(INTEL_SYNTAX_INS);
455446
}
456-
447+
457448
// 4. Generate Extended Asm block
458449

459450
let block = self.llbb();
@@ -472,7 +463,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
472463
}
473464

474465
if !options.contains(InlineAsmOptions::PRESERVES_FLAGS) {
475-
// TODO(@Commeownist): I'm not 100% sure this one clobber is sufficient
466+
// TODO(@Commeownist): I'm not 100% sure this one clobber is sufficient
476467
// on all architectures. For instance, what about FP stack?
477468
extended_asm.add_clobber("cc");
478469
}
@@ -491,18 +482,18 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
491482
self.call(self.type_void(), builtin_unreachable, &[], None);
492483
}
493484

494-
// Write results to outputs.
485+
// Write results to outputs.
495486
//
496487
// We need to do this because:
497-
// 1. Turning `PlaceRef` into `RValue` is error-prone and has nasty edge cases
488+
// 1. Turning `PlaceRef` into `RValue` is error-prone and has nasty edge cases
498489
// (especially with current `rustc_backend_ssa` API).
499490
// 2. Not every output operand has an `out_place`, and it's required by `add_output_operand`.
500491
//
501492
// Instead, we generate a temporary output variable for each output operand, and then this loop,
502493
// generates `out_place = tmp_var;` assignments if out_place exists.
503494
for op in &outputs {
504495
if let Some(place) = op.out_place {
505-
OperandValue::Immediate(op.tmp_var.to_rvalue()).store(self, place);
496+
OperandValue::Immediate(op.tmp_var.to_rvalue()).store(self, place);
506497
}
507498
}
508499

tests/run/asm.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@ extern "C" {
1717
fn add_asm(a: i64, b: i64) -> i64;
1818
}
1919

20+
pub unsafe fn mem_cpy(dst: *mut u8, src: *const u8, len: usize) {
21+
asm!(
22+
"rep movsb",
23+
inout("rdi") dst => _,
24+
inout("rsi") src => _,
25+
inout("rcx") len => _,
26+
options(preserves_flags, nostack)
27+
);
28+
}
29+
2030
fn main() {
2131
unsafe {
2232
asm!("nop");
@@ -62,11 +72,11 @@ fn main() {
6272
}
6373
assert_eq!(x, 43);
6474

65-
// check inout(reg_class) x
75+
// check inout(reg_class) x
6676
let mut x: u64 = 42;
6777
unsafe {
6878
asm!("add {0}, {0}",
69-
inout(reg) x
79+
inout(reg) x
7080
);
7181
}
7282
assert_eq!(x, 84);
@@ -75,7 +85,7 @@ fn main() {
7585
let mut x: u64 = 42;
7686
unsafe {
7787
asm!("add r11, r11",
78-
inout("r11") x
88+
inout("r11") x
7989
);
8090
}
8191
assert_eq!(x, 84);
@@ -98,12 +108,12 @@ fn main() {
98108
assert_eq!(res, 7);
99109
assert_eq!(rem, 2);
100110

101-
// check const
111+
// check const
102112
let mut x: u64 = 42;
103113
unsafe {
104114
asm!("add {}, {}",
105115
inout(reg) x,
106-
const 1
116+
const 1
107117
);
108118
}
109119
assert_eq!(x, 43);
@@ -150,4 +160,11 @@ fn main() {
150160
assert_eq!(x, 42);
151161

152162
assert_eq!(unsafe { add_asm(40, 2) }, 42);
163+
164+
let array1 = [1u8, 2, 3];
165+
let mut array2 = [0u8, 0, 0];
166+
unsafe {
167+
mem_cpy(array2.as_mut_ptr(), array1.as_ptr(), 3);
168+
}
169+
assert_eq!(array1, array2);
153170
}

0 commit comments

Comments
 (0)