From fb2c0623d2e0c628291a03b365bb26b6f7b631cc Mon Sep 17 00:00:00 2001 From: Commeownist Date: Fri, 20 Aug 2021 17:12:55 +0300 Subject: [PATCH 01/24] Exclude junk files from the repo --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index d9f0ae05ed4..1e2f9e3aebb 100644 --- a/.gitignore +++ b/.gitignore @@ -7,11 +7,14 @@ perf.data.old *.events *.string* /build_sysroot/sysroot +/build_sysroot/sysroot_src /build_sysroot/Cargo.lock /build_sysroot/test_target/Cargo.lock /rust +/simple-raytracer /regex gimple* *asm res test-backend +gcc_path From 02f04e60ff7bc4c592c0b0756029f7c05cdfd943 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 21 Aug 2021 10:41:42 +0300 Subject: [PATCH 02/24] Handle gcc_path properly --- build.sh | 7 ++++++- config.sh | 7 ++++++- gcc_path | 0 test.sh | 7 ++++++- 4 files changed, 18 insertions(+), 3 deletions(-) delete mode 100644 gcc_path diff --git a/build.sh b/build.sh index 9f1228687e2..17a0d2ab3f0 100755 --- a/build.sh +++ b/build.sh @@ -3,7 +3,12 @@ #set -x set -e -export GCC_PATH=$(cat gcc_path) +if [ -f ./gcc_path ]; then + export GCC_PATH=$(cat gcc_path) +else + echo 'Please put the path to your custom build of libgccjit in the file `gcc_path`, see Readme.md for details' + exit 1 +fi export LD_LIBRARY_PATH="$GCC_PATH" export LIBRARY_PATH="$GCC_PATH" diff --git a/config.sh b/config.sh index bd2d915f589..3ef9f11668a 100644 --- a/config.sh +++ b/config.sh @@ -2,7 +2,12 @@ set -e export CARGO_INCREMENTAL=0 -export GCC_PATH=$(cat gcc_path) +if [ -f ./gcc_path ]; then + export GCC_PATH=$(cat gcc_path) +else + echo 'Please put the path to your custom build of libgccjit in the file `gcc_path`, see Readme.md for details' + exit 1 +fi unamestr=`uname` if [[ "$unamestr" == 'Linux' ]]; then diff --git a/gcc_path b/gcc_path deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/test.sh b/test.sh index 06faa98d9b1..c246d8eacc5 100755 --- a/test.sh +++ b/test.sh @@ -4,7 +4,12 @@ set -e -export GCC_PATH=$(cat gcc_path) +if [ -f ./gcc_path ]; then + export GCC_PATH=$(cat gcc_path) +else + echo 'Please put the path to your custom build of libgccjit in the file `gcc_path`, see Readme.md for details' + exit 1 +fi export LD_LIBRARY_PATH="$GCC_PATH" export LIBRARY_PATH="$GCC_PATH" From 59c009f507b457b197b33b2d158ba3755ec78448 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Mon, 23 Aug 2021 13:28:45 +0300 Subject: [PATCH 03/24] Implement basic support for inline assembly --- src/asm.rs | 597 +++++++++++++++++++++++++++++++++++------------------ src/lib.rs | 2 +- 2 files changed, 401 insertions(+), 198 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index e4d57c39de4..f0239286b00 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -1,148 +1,356 @@ -use gccjit::{RValue, ToRValue, Type}; +use gccjit::{LValue, RValue, ToRValue, Type}; use rustc_ast::ast::{InlineAsmOptions, InlineAsmTemplatePiece}; use rustc_codegen_ssa::mir::operand::OperandValue; use rustc_codegen_ssa::mir::place::PlaceRef; -use rustc_codegen_ssa::traits::{AsmBuilderMethods, AsmMethods, BaseTypeMethods, BuilderMethods, GlobalAsmOperandRef, InlineAsmOperandRef}; -use rustc_data_structures::fx::FxHashMap; +use rustc_codegen_ssa::traits::{AsmBuilderMethods, AsmMethods, BaseTypeMethods, BuilderMethods, GlobalAsmOperandRef, InlineAsmOperandRef, MiscMethods}; + use rustc_hir::LlvmInlineAsmInner; use rustc_middle::bug; use rustc_span::Span; use rustc_target::asm::*; +use std::borrow::Cow; + use crate::builder::Builder; use crate::context::CodegenCx; use crate::type_of::LayoutGccExt; + +// Rust asm! and GCC Extended Asm semantics differ substantially. +// +// 1. Rust asm operands go along as one list of operands. Operands themselves indicate +// if they're "in" or "out". "In" and "out" operands can interleave. One operand can be +// both "in" and "out" (`inout(reg)`). +// +// GCC asm has two different lists for "in" and "out" operands. In terms of gccjit, +// this means that all "out" operands must go before "in" operands. "In" and "out" operands +// cannot interleave. +// +// 2. Operand lists in both Rust and GCC are indexed. Index starts from 0. Indexes are important +// because the asm template refers to operands by index. +// +// Mapping from Rust to GCC index would be 1-1 if it wasn't for... +// +// 3. Clobbers. GCC has a separate list of clobbers, and clobbers don't have indexes. +// Contrary, Rust expresses clobbers through "out" operands that aren't tied to +// a variable (`_`), and such "clobbers" do have index. +// +// 4. Furthermore, GCC Extended Asm does not support explicit register constraints +// (like `out("eax")`) directly, offering so-called "local register variables" +// as a workaround. This variables need to be declared and initialized *before* +// the Extended Asm block but *after* normal local variables. +// +// With that in mind, let's see how we translate Rust syntax to GCC +// (from now on, `CC` stands for "constraint code"): +// +// * `out(reg_class) var` -> translated to output operand: `"=CC"(var)` +// * `in(reg_class) var` -> translated to input operand: `"CC"(var)` +// +// * `out(reg_class) _` -> translated to one `=r(tmp)`, where "tmp" is a temporary unused variable +// +// * `out("explicit register") _` -> not translated to any operands, register is simply added to clobbers list +// +// * `inout(reg_class) in_var => out_var` -> translated to two operands: +// output: `"+CC"(in_var)` +// input: `"num"(out_var)` where num is the GCC index +// of the corresponding output operand +// +// * `inout(reg_class) in_var => _` -> same as `inout(reg_class) in_var => tmp`, +// where "tmp" is a temporary unused variable +// +// * `out/in/inout("explicit register") var` -> translated to one or two operands as described above +// with `"r"(var)` constraint, +// and one register variable assigned to the desired register. +// + +struct AsmOutOperand<'a, 'tcx, 'gcc> { + rust_idx: usize, + constraint: &'a str, + late: bool, + readwrite: bool, + + tmp_var: LValue<'gcc>, + out_place: Option>> +} + +struct AsmInOperand<'a, 'tcx> { + rust_idx: usize, + constraint: Cow<'a, str>, + val: RValue<'tcx> +} + +impl AsmOutOperand<'_, '_, '_> { + fn to_constraint(&self) -> String { + let mut res = String::with_capacity(self.constraint.len() + self.late as usize + 1); + + res.push(if self.readwrite { '+' } else { '=' }); + if self.late { + res.push('&'); + } + + res.push_str(&self.constraint); + res + } +} + + impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { - fn codegen_llvm_inline_asm(&mut self, _ia: &LlvmInlineAsmInner, _outputs: Vec>>, mut _inputs: Vec>, _span: Span) -> bool { - // TODO(antoyo) + fn codegen_llvm_inline_asm(&mut self, _ia: &LlvmInlineAsmInner, _outputs: Vec>>, mut _inputs: Vec>, span: Span) -> bool { + // TODO(@Commeownist): switch to `struct_span_err_with_code` + // once we get this merged into rustc + self.sess().struct_span_err(span, "GCC backend does not support `llvm_asm!`") + .help("consider using the `asm!` macro instead") + .emit(); + + // We return `true` even though we've failed to generate the asm + // because we want to suppress the "malformed inline assembly" error + // generated by the frontend. return true; } - fn codegen_inline_asm(&mut self, template: &[InlineAsmTemplatePiece], operands: &[InlineAsmOperandRef<'tcx, Self>], options: InlineAsmOptions, _span: &[Span]) { + fn codegen_inline_asm(&mut self, template: &[InlineAsmTemplatePiece], rust_operands: &[InlineAsmOperandRef<'tcx, Self>], options: InlineAsmOptions, span: &[Span]) { let asm_arch = self.tcx.sess.asm_arch.unwrap(); + let att_dialect = matches!(asm_arch, InlineAsmArch::X86 | InlineAsmArch::X86_64) && + options.contains(InlineAsmOptions::ATT_SYNTAX); + let intel_dialect = matches!(asm_arch, InlineAsmArch::X86 | InlineAsmArch::X86_64) && + !options.contains(InlineAsmOptions::ATT_SYNTAX); - let intel_dialect = - match asm_arch { - InlineAsmArch::X86 | InlineAsmArch::X86_64 if !options.contains(InlineAsmOptions::ATT_SYNTAX) => true, - _ => false, - }; + // GCC index of an output operand equals its position in the array + let mut outputs = vec![]; + + // GCC index of an input operand equals its position in the array + // added to `outputs.len()` + let mut inputs = vec![]; + + // Clobbers collected from `out("explicit register") _` and `inout("expl_reg") var => _` + let mut clobbers = vec![]; - // Collect the types of output operands - // FIXME(antoyo): we do this here instead of later because of a bug in libgccjit where creating the - // variable after the extended asm expression causes a segfault: - // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100380 - let mut output_vars = FxHashMap::default(); - let mut operand_numbers = FxHashMap::default(); - let mut current_number = 0; - for (idx, op) in operands.iter().enumerate() { + // We're trying to preallocate space for the template + let mut constants_len = 0; + + // There are rules we must adhere to if we want GCC to do the right thing: + // + // * Every local variable the asm block uses as output must be declared *before* + // the asm block. + // * There must be no instructions whatsoever between the register variables and the asm. + // + // Therefore, the backend must generate the instructions strictly in this order: + // + // 1. Output variables. + // 2. Register variables. + // 3. The asm block. + // + // We also must make sure that no input operands are emitted before output operands. + // + // This is why we work in passes, first emitting local vars, then local register vars. + // Also, we don't emit any asm operands immediately; we save them to + // the one of the buffers to be emitted later. + + for (rust_idx, op) in rust_operands.iter().enumerate() { match *op { - InlineAsmOperandRef::Out { place, .. } => { - let ty = - match place { - Some(place) => place.layout.gcc_type(self.cx, false), - None => { - // If the output is discarded, we don't really care what - // type is used. We're just using this to tell GCC to - // reserve the register. - //dummy_output_type(self.cx, reg.reg_class()) - - // NOTE: if no output value, we should not create one (it will be a - // clobber). - continue; - }, - }; - let var = self.current_func().new_local(None, ty, "output_register"); - operand_numbers.insert(idx, current_number); - current_number += 1; - output_vars.insert(idx, var); + InlineAsmOperandRef::Out { reg, late, place } => { + let (constraint, ty) = match (reg_to_gcc(reg), place) { + (Ok(constraint), Some(place)) => (constraint, place.layout.gcc_type(self.cx, false)), + (Ok(constraint), None) => (constraint, dummy_output_type(self.cx, reg.reg_class())), + (Err(_), Some(_)) => { + // left for the next pass + continue + }, + (Err(reg_name), None) => { + clobbers.push(reg_name); + continue + } + }; + + let tmp_var = self.current_func().new_local(None, ty, "output_register"); + + outputs.push(AsmOutOperand { + constraint, + rust_idx, + late, + readwrite: false, + tmp_var, + out_place: place + }); + } + + InlineAsmOperandRef::In { reg, value } => { + if let Ok(constraint) = reg_to_gcc(reg).map(Cow::Borrowed) { + inputs.push(AsmInOperand { + constraint, + rust_idx, + val: value.immediate() + }); + } else { + // left for the next pass + continue + } } - InlineAsmOperandRef::InOut { out_place, .. } => { - let ty = - match out_place { - Some(place) => place.layout.gcc_type(self.cx, false), - None => { - // NOTE: if no output value, we should not create one. - continue; - }, - }; - operand_numbers.insert(idx, current_number); - current_number += 1; - let var = self.current_func().new_local(None, ty, "output_register"); - output_vars.insert(idx, var); + + InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => { + let constraint = if let Ok(constarint) = reg_to_gcc(reg) { + constarint + } else { + // left for the next pass + continue + }; + + // Rustc frontend guarantees that input and output types are "compatible", + // so we can just use input var's type for the output variable. + // + // This decision is also backed by the fact that LLVM needs in and out + // values to be of *exactly the same type*, not just "compatible". + // I'm not sure if GCC is so picky too, but better safe than sorry. + let ty = in_value.layout.gcc_type(self.cx, false); + let tmp_var = self.current_func().new_local(None, ty, "output_register"); + outputs.push(AsmOutOperand { + constraint, + rust_idx, + late, + readwrite: true, + tmp_var, + out_place, + }); + + // "in" operand to be emitted later + let out_gcc_idx = outputs.len() - 1; + let constraint = Cow::Owned(out_gcc_idx.to_string()); + + inputs.push(AsmInOperand { + constraint, + rust_idx, + val: in_value.immediate() + }); + } + + InlineAsmOperandRef::Const { ref string } => { + constants_len += string.len() + att_dialect as usize; + } + + InlineAsmOperandRef::SymFn { instance } => { + inputs.push(AsmInOperand { + constraint: Cow::Borrowed("s"), + rust_idx, + val: self.cx.get_fn(instance) + }); + } + + InlineAsmOperandRef::SymStatic { def_id } => { + inputs.push(AsmInOperand { + constraint: Cow::Borrowed("s"), + rust_idx, + val: self.cx.get_static(def_id) + }); } - _ => {} } } - // All output operands must come before the input operands, hence the 2 loops. - for (idx, op) in operands.iter().enumerate() { + for (rust_idx, op) in rust_operands.iter().enumerate() { + let not_yet = |reg_name| { + // TODO(@Commeownist) + let err = format!("GCC backend does not support explicit registers such as `{}` just yet", reg_name); + self.sess().struct_span_err(span[rust_idx], &err) + .note("need register variables support in libgccjit") + .emit(); + }; + match *op { - InlineAsmOperandRef::In { .. } | InlineAsmOperandRef::InOut { .. } => { - operand_numbers.insert(idx, current_number); - current_number += 1; - }, - _ => (), + // `out("explicit register") _` + InlineAsmOperandRef::Out { reg, late:_, place:_ } => { + if let Err(reg_name) = reg_to_gcc(reg) { + not_yet(reg_name); + } + + // processed in the previous pass + } + + // `in("explicit register") var` + InlineAsmOperandRef::In { reg, value:_ } => { + if let Err(reg_name) = reg_to_gcc(reg) { + not_yet(reg_name); + } + + // processed in the previous pass + } + + // `inout("explicit register") in_var => out_var/_` + InlineAsmOperandRef::InOut { reg, late:_, in_value:_, out_place:_ } => { + if let Err(reg_name) = reg_to_gcc(reg) { + not_yet(reg_name); + } + + // processed in the previous pass + } + + InlineAsmOperandRef::Const { .. } + | InlineAsmOperandRef::SymFn { .. } + | InlineAsmOperandRef::SymStatic { .. } => { + // processed on the previous pass + } } } // Build the template string - let mut template_str = String::new(); + let mut template_str = String::with_capacity(estimate_template_length(template, constants_len)); for piece in template { match *piece { InlineAsmTemplatePiece::String(ref string) => { - if string.contains('%') { - for c in string.chars() { - if c == '%' { - template_str.push_str("%%"); - } - else { - template_str.push(c); - } - } - } - else { - template_str.push_str(string) + for s in string.split('%').intersperse("%%") { + template_str.push_str(s); } } InlineAsmTemplatePiece::Placeholder { operand_idx, modifier, span: _ } => { - match operands[operand_idx] { - InlineAsmOperandRef::Out { reg, place: Some(_), .. } => { + let mut push_to_template = |modifier, gcc_idx| { + use std::fmt::Write; + + template_str.push('%'); + if let Some(modifier) = modifier { + template_str.push(modifier); + } + write!(template_str, "{}", gcc_idx).unwrap(); + }; + + match rust_operands[operand_idx] { + InlineAsmOperandRef::Out { reg, .. } => { let modifier = modifier_to_gcc(asm_arch, reg.reg_class(), modifier); - if let Some(modifier) = modifier { - template_str.push_str(&format!("%{}{}", modifier, operand_numbers[&operand_idx])); - } else { - template_str.push_str(&format!("%{}", operand_numbers[&operand_idx])); - } - }, - InlineAsmOperandRef::Out { place: None, .. } => { - unimplemented!("Out None"); - }, - InlineAsmOperandRef::In { reg, .. } - | InlineAsmOperandRef::InOut { reg, .. } => { + let gcc_index = outputs.binary_search_by(|op| operand_idx.cmp(&op.rust_idx)).unwrap(); + push_to_template(modifier, gcc_index); + } + + InlineAsmOperandRef::In { reg, .. } => { let modifier = modifier_to_gcc(asm_arch, reg.reg_class(), modifier); - if let Some(modifier) = modifier { - template_str.push_str(&format!("%{}{}", modifier, operand_numbers[&operand_idx])); - } else { - template_str.push_str(&format!("%{}", operand_numbers[&operand_idx])); - } + let in_gcc_index = inputs.binary_search_by(|op| operand_idx.cmp(&op.rust_idx)).unwrap(); + let gcc_index = in_gcc_index + outputs.len(); + push_to_template(modifier, gcc_index); + } + + InlineAsmOperandRef::SymFn { .. } + | InlineAsmOperandRef::SymStatic { .. } => { + let in_gcc_index = inputs.binary_search_by(|op| operand_idx.cmp(&op.rust_idx)).unwrap(); + let gcc_index = in_gcc_index + outputs.len(); + push_to_template(None, gcc_index); } + + InlineAsmOperandRef::InOut { reg, .. } => { + let modifier = modifier_to_gcc(asm_arch, reg.reg_class(), modifier); + + // The input register is tied to the input, so we can just use the index of the output register + let gcc_index = outputs.binary_search_by(|op| operand_idx.cmp(&op.rust_idx)).unwrap(); + push_to_template(modifier, gcc_index); + } + InlineAsmOperandRef::Const { ref string } => { // Const operands get injected directly into the template + if att_dialect { + template_str.push('$'); + } template_str.push_str(string); } - InlineAsmOperandRef::SymFn { .. } - | InlineAsmOperandRef::SymStatic { .. } => { - unimplemented!(); - // Only emit the raw symbol name - //template_str.push_str(&format!("${{{}:c}}", op_idx[&operand_idx])); - } } } } } - let block = self.llbb(); let template_str = if intel_dialect { template_str @@ -150,105 +358,90 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { else { // FIXME(antoyo): this might break the "m" memory constraint: // https://stackoverflow.com/a/9347957/389119 - // TODO(antoyo): only set on x86 platforms. format!(".att_syntax noprefix\n\t{}\n\t.intel_syntax noprefix", template_str) }; + + let block = self.llbb(); let extended_asm = block.add_extended_asm(None, &template_str); - // Collect the types of output operands - let mut output_types = vec![]; - for (idx, op) in operands.iter().enumerate() { - match *op { - InlineAsmOperandRef::Out { reg, late, place } => { - let ty = - match place { - Some(place) => place.layout.gcc_type(self.cx, false), - None => { - // If the output is discarded, we don't really care what - // type is used. We're just using this to tell GCC to - // reserve the register. - dummy_output_type(self.cx, reg.reg_class()) - }, - }; - output_types.push(ty); - let prefix = if late { "=" } else { "=&" }; - let constraint = format!("{}{}", prefix, reg_to_gcc(reg)); - - if place.is_some() { - let var = output_vars[&idx]; - extended_asm.add_output_operand(None, &constraint, var); - } - else { - // NOTE: reg.to_string() returns the register name with quotes around it so - // remove them. - extended_asm.add_clobber(reg.to_string().trim_matches('"')); - } - } - InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => { - let ty = - match out_place { - Some(out_place) => out_place.layout.gcc_type(self.cx, false), - None => dummy_output_type(self.cx, reg.reg_class()) - }; - output_types.push(ty); - // TODO(antoyo): prefix of "+" for reading and writing? - let prefix = if late { "=" } else { "=&" }; - let constraint = format!("{}{}", prefix, reg_to_gcc(reg)); - - if out_place.is_some() { - let var = output_vars[&idx]; - // TODO(antoyo): also specify an output operand when out_place is none: that would - // be the clobber but clobbers do not support general constraint like reg; - // they only support named registers. - // Not sure how we can do this. And the LLVM backend does not seem to add a - // clobber. - extended_asm.add_output_operand(None, &constraint, var); - } + for op in &outputs { + extended_asm.add_output_operand(None, &op.to_constraint(), op.tmp_var); + } - let constraint = reg_to_gcc(reg); - extended_asm.add_input_operand(None, &constraint, in_value.immediate()); - } - InlineAsmOperandRef::In { reg, value } => { - let constraint = reg_to_gcc(reg); - extended_asm.add_input_operand(None, &constraint, value.immediate()); - } - _ => {} - } + for op in &inputs { + extended_asm.add_input_operand(None, &op.constraint, op.val); + } + + for clobber in clobbers.iter() { + extended_asm.add_clobber(clobber); + } + + if !options.contains(InlineAsmOptions::PRESERVES_FLAGS) { + // TODO(@Commeownist): I'm not 100% sure this one clobber is sufficient + // on all architectures. For instance, what about FP stack? + extended_asm.add_clobber("cc"); + } + if !options.contains(InlineAsmOptions::NOMEM) { + extended_asm.add_clobber("memory"); + } + if !options.contains(InlineAsmOptions::PURE) { + extended_asm.set_volatile_flag(true); + } + if !options.contains(InlineAsmOptions::NOSTACK) { + // TODO(@Commeownist): figure out how to align stack } // Write results to outputs - for (idx, op) in operands.iter().enumerate() { - if let InlineAsmOperandRef::Out { place: Some(place), .. } - | InlineAsmOperandRef::InOut { out_place: Some(place), .. } = *op - { - OperandValue::Immediate(output_vars[&idx].to_rvalue()).store(self, place); + for op in &outputs { + if let Some(place) = op.out_place { + OperandValue::Immediate(op.tmp_var.to_rvalue()).store(self, place); } } + + // TODO(@Commeownist): maybe we need to follow up with a call to `__builtin_unreachable` + // if NORETURN option is set. + // + // In fact, GCC docs for __builtin_unreachable do describe it as a good fit after diverging asm blocks! + // https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable } } +fn estimate_template_length(template: &[InlineAsmTemplatePiece], constants_len: usize) -> usize { + let len: usize = template.iter().map(|piece| { + match *piece { + InlineAsmTemplatePiece::String(ref string) => { + string.len() + } + InlineAsmTemplatePiece::Placeholder { .. } => { + // '%' + 1 char modifier + 2 chars index + 4 + } + } + }) + .sum(); + + // increase it by 5% to account for possible '%' signs that'll be duplicated + // I pulled the number out of blue, but should be fair enough + // as the upper bound + (len as f32 * 1.05) as usize + constants_len +} + /// Converts a register class to a GCC constraint code. -// TODO(antoyo): return &'static str instead? -fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> String { - match reg { +fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> Result<&'static str, &'static str> { + let constraint = match reg { // For vector registers LLVM wants the register name to match the type size. InlineAsmRegOrRegClass::Reg(reg) => { // TODO(antoyo): add support for vector register. - let constraint = - match reg.name() { - "ax" => "a", - "bx" => "b", - "cx" => "c", - "dx" => "d", - "si" => "S", - "di" => "D", - // TODO(antoyo): for registers like r11, we have to create a register variable: https://stackoverflow.com/a/31774784/389119 - // TODO(antoyo): in this case though, it's a clobber, so it should work as r11. - // Recent nightly supports clobber() syntax, so update to it. It does not seem - // like it's implemented yet. - name => name, // FIXME(antoyo): probably wrong. - }; - constraint.to_string() + match reg.name() { + "ax" => "a", + "bx" => "b", + "cx" => "c", + "dx" => "d", + "si" => "S", + "di" => "D", + // For registers like r11, we have to create a register variable: https://stackoverflow.com/a/31774784/389119 + name => return Err(name), + } }, InlineAsmRegOrRegClass::RegClass(reg) => match reg { InlineAsmRegClass::AArch64(AArch64InlineAsmRegClass::preg) => unimplemented!(), @@ -278,23 +471,25 @@ fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> String { InlineAsmRegClass::RiscV(RiscVInlineAsmRegClass::reg) => unimplemented!(), InlineAsmRegClass::RiscV(RiscVInlineAsmRegClass::freg) => unimplemented!(), InlineAsmRegClass::RiscV(RiscVInlineAsmRegClass::vreg) => unimplemented!(), - InlineAsmRegClass::X86(X86InlineAsmRegClass::mmx_reg) => unimplemented!(), InlineAsmRegClass::X86(X86InlineAsmRegClass::reg) => "r", - InlineAsmRegClass::X86(X86InlineAsmRegClass::reg_abcd) => unimplemented!(), - InlineAsmRegClass::X86(X86InlineAsmRegClass::reg_byte) => unimplemented!(), + InlineAsmRegClass::X86(X86InlineAsmRegClass::reg_abcd) => "Q", + InlineAsmRegClass::X86(X86InlineAsmRegClass::reg_byte) => "q", InlineAsmRegClass::X86(X86InlineAsmRegClass::xmm_reg) - | InlineAsmRegClass::X86(X86InlineAsmRegClass::ymm_reg) => unimplemented!(), - InlineAsmRegClass::X86(X86InlineAsmRegClass::x87_reg) => unimplemented!(), - InlineAsmRegClass::X86(X86InlineAsmRegClass::zmm_reg) => unimplemented!(), + | InlineAsmRegClass::X86(X86InlineAsmRegClass::ymm_reg) => "x", + InlineAsmRegClass::X86(X86InlineAsmRegClass::zmm_reg) => "v", InlineAsmRegClass::X86(X86InlineAsmRegClass::kreg) => unimplemented!(), InlineAsmRegClass::Wasm(WasmInlineAsmRegClass::local) => unimplemented!(), + InlineAsmRegClass::X86( + X86InlineAsmRegClass::x87_reg | X86InlineAsmRegClass::mmx_reg, + ) => unreachable!("clobber-only"), InlineAsmRegClass::SpirV(SpirVInlineAsmRegClass::reg) => { bug!("GCC backend does not support SPIR-V") } InlineAsmRegClass::Err => unreachable!(), } - .to_string(), - } + }; + + Ok(constraint) } /// Type to use for outputs that are discarded. It doesn't really matter what @@ -379,7 +574,7 @@ impl<'gcc, 'tcx> AsmMethods for CodegenCx<'gcc, 'tcx> { match operands[operand_idx] { GlobalAsmOperandRef::Const { ref string } => { // Const operands get injected directly into the - // template. Note that we don't need to escape $ + // template. Note that we don't need to escape % // here unlike normal inline assembly. template_str.push_str(string); } @@ -431,8 +626,7 @@ fn modifier_to_gcc(arch: InlineAsmArch, reg: InlineAsmRegClass, modifier: Option InlineAsmRegClass::RiscV(RiscVInlineAsmRegClass::vreg) => unimplemented!(), InlineAsmRegClass::X86(X86InlineAsmRegClass::reg) | InlineAsmRegClass::X86(X86InlineAsmRegClass::reg_abcd) => match modifier { - None if arch == InlineAsmArch::X86_64 => Some('q'), - None => Some('k'), + None => if arch == InlineAsmArch::X86_64 { Some('q') } else { Some('k') }, Some('l') => Some('b'), Some('h') => Some('h'), Some('x') => Some('w'), @@ -440,13 +634,22 @@ fn modifier_to_gcc(arch: InlineAsmArch, reg: InlineAsmRegClass, modifier: Option Some('r') => Some('q'), _ => unreachable!(), }, - InlineAsmRegClass::X86(X86InlineAsmRegClass::mmx_reg) => unimplemented!(), - InlineAsmRegClass::X86(X86InlineAsmRegClass::reg_byte) => unimplemented!(), - InlineAsmRegClass::X86(X86InlineAsmRegClass::xmm_reg) - | InlineAsmRegClass::X86(X86InlineAsmRegClass::ymm_reg) - | InlineAsmRegClass::X86(X86InlineAsmRegClass::zmm_reg) => unimplemented!(), - InlineAsmRegClass::X86(X86InlineAsmRegClass::x87_reg) => unimplemented!(), - InlineAsmRegClass::X86(X86InlineAsmRegClass::kreg) => unimplemented!(), + InlineAsmRegClass::X86(X86InlineAsmRegClass::reg_byte) => None, + InlineAsmRegClass::X86(reg @ X86InlineAsmRegClass::xmm_reg) + | InlineAsmRegClass::X86(reg @ X86InlineAsmRegClass::ymm_reg) + | InlineAsmRegClass::X86(reg @ X86InlineAsmRegClass::zmm_reg) => match (reg, modifier) { + (X86InlineAsmRegClass::xmm_reg, None) => Some('x'), + (X86InlineAsmRegClass::ymm_reg, None) => Some('t'), + (X86InlineAsmRegClass::zmm_reg, None) => Some('g'), + (_, Some('x')) => Some('x'), + (_, Some('y')) => Some('t'), + (_, Some('z')) => Some('g'), + _ => unreachable!(), + }, + InlineAsmRegClass::X86(X86InlineAsmRegClass::kreg) => None, + InlineAsmRegClass::X86(X86InlineAsmRegClass::x87_reg | X86InlineAsmRegClass::mmx_reg) => { + unreachable!("clobber-only") + } InlineAsmRegClass::Wasm(WasmInlineAsmRegClass::local) => unimplemented!(), InlineAsmRegClass::SpirV(SpirVInlineAsmRegClass::reg) => { bug!("LLVM backend does not support SPIR-V") diff --git a/src/lib.rs b/src/lib.rs index 6febccff1ff..bb6bf41facd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,7 +5,7 @@ * TODO(antoyo): remove the patches. */ -#![feature(rustc_private, decl_macro, associated_type_bounds, never_type, trusted_len)] +#![feature(rustc_private, decl_macro, associated_type_bounds, never_type, trusted_len, iter_intersperse)] #![allow(broken_intra_doc_links)] #![recursion_limit="256"] #![warn(rust_2018_idioms)] From 9d3920adc73bb2c9e56d76d3c034de61d64479fd Mon Sep 17 00:00:00 2001 From: Commeownist Date: Mon, 23 Aug 2021 23:38:46 +0300 Subject: [PATCH 04/24] Fix some bugs --- src/asm.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index f0239286b00..2d572c875eb 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -84,7 +84,7 @@ impl AsmOutOperand<'_, '_, '_> { let mut res = String::with_capacity(self.constraint.len() + self.late as usize + 1); res.push(if self.readwrite { '+' } else { '=' }); - if self.late { + if !self.late { res.push('&'); } @@ -248,7 +248,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { for (rust_idx, op) in rust_operands.iter().enumerate() { let not_yet = |reg_name| { // TODO(@Commeownist) - let err = format!("GCC backend does not support explicit registers such as `{}` just yet", reg_name); + let err = format!("GCC backend does not support explicit registers such as `\"{}\"` just yet", reg_name); self.sess().struct_span_err(span[rust_idx], &err) .note("need register variables support in libgccjit") .emit(); @@ -313,20 +313,20 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { match rust_operands[operand_idx] { InlineAsmOperandRef::Out { reg, .. } => { let modifier = modifier_to_gcc(asm_arch, reg.reg_class(), modifier); - let gcc_index = outputs.binary_search_by(|op| operand_idx.cmp(&op.rust_idx)).unwrap(); + let gcc_index = outputs.iter().position(|op| operand_idx == op.rust_idx).unwrap(); push_to_template(modifier, gcc_index); } InlineAsmOperandRef::In { reg, .. } => { let modifier = modifier_to_gcc(asm_arch, reg.reg_class(), modifier); - let in_gcc_index = inputs.binary_search_by(|op| operand_idx.cmp(&op.rust_idx)).unwrap(); + let in_gcc_index = inputs.iter().position(|op| operand_idx == op.rust_idx).unwrap(); let gcc_index = in_gcc_index + outputs.len(); push_to_template(modifier, gcc_index); } InlineAsmOperandRef::SymFn { .. } | InlineAsmOperandRef::SymStatic { .. } => { - let in_gcc_index = inputs.binary_search_by(|op| operand_idx.cmp(&op.rust_idx)).unwrap(); + let in_gcc_index = inputs.iter().position(|op| operand_idx == op.rust_idx).unwrap(); let gcc_index = in_gcc_index + outputs.len(); push_to_template(None, gcc_index); } @@ -335,7 +335,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { let modifier = modifier_to_gcc(asm_arch, reg.reg_class(), modifier); // The input register is tied to the input, so we can just use the index of the output register - let gcc_index = outputs.binary_search_by(|op| operand_idx.cmp(&op.rust_idx)).unwrap(); + let gcc_index = outputs.iter().position(|op| operand_idx == op.rust_idx).unwrap(); push_to_template(modifier, gcc_index); } From d0b5c4aa66737d8adfdea5494888d5aa0f0b9c1c Mon Sep 17 00:00:00 2001 From: Commeownist Date: Tue, 24 Aug 2021 11:12:52 +0300 Subject: [PATCH 05/24] Review requests --- src/asm.rs | 40 ++++++++++++++++++++++++++++++---------- src/lib.rs | 2 +- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index 2d572c875eb..1413481f504 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -83,7 +83,8 @@ impl AsmOutOperand<'_, '_, '_> { fn to_constraint(&self) -> String { let mut res = String::with_capacity(self.constraint.len() + self.late as usize + 1); - res.push(if self.readwrite { '+' } else { '=' }); + let sign = if self.readwrite { '+' } else { '=' }; + res.push(sign); if !self.late { res.push('&'); } @@ -151,6 +152,10 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { InlineAsmOperandRef::Out { reg, late, place } => { let (constraint, ty) = match (reg_to_gcc(reg), place) { (Ok(constraint), Some(place)) => (constraint, place.layout.gcc_type(self.cx, false)), + // When `reg` is a class and not an explicit register but the out place is not specified, + // we need to create an unused output variable to assign the output to. This var + // needs to be of a type that's "compatible" with the register class, but specific type + // doesn't matter. (Ok(constraint), None) => (constraint, dummy_output_type(self.cx, reg.reg_class())), (Err(_), Some(_)) => { // left for the next pass @@ -188,8 +193,8 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => { - let constraint = if let Ok(constarint) = reg_to_gcc(reg) { - constarint + let constraint = if let Ok(constraint) = reg_to_gcc(reg) { + constraint } else { // left for the next pass continue @@ -295,7 +300,14 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { for piece in template { match *piece { InlineAsmTemplatePiece::String(ref string) => { - for s in string.split('%').intersperse("%%") { + // TODO(@Commeownist): switch to `Iterator::intersperse` once it's stable + let mut iter = string.split('%'); + if let Some(s) = iter.next() { + template_str.push_str(s); + } + + for s in iter { + template_str.push_str("%%"); template_str.push_str(s); } } @@ -307,26 +319,32 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { if let Some(modifier) = modifier { template_str.push(modifier); } - write!(template_str, "{}", gcc_idx).unwrap(); + write!(template_str, "{}", gcc_idx).expect("pushing to string failed"); }; match rust_operands[operand_idx] { InlineAsmOperandRef::Out { reg, .. } => { let modifier = modifier_to_gcc(asm_arch, reg.reg_class(), modifier); - let gcc_index = outputs.iter().position(|op| operand_idx == op.rust_idx).unwrap(); + let gcc_index = outputs.iter() + .position(|op| operand_idx == op.rust_idx) + .expect("wrong rust index"); push_to_template(modifier, gcc_index); } InlineAsmOperandRef::In { reg, .. } => { let modifier = modifier_to_gcc(asm_arch, reg.reg_class(), modifier); - let in_gcc_index = inputs.iter().position(|op| operand_idx == op.rust_idx).unwrap(); + let in_gcc_index = inputs.iter() + .position(|op| operand_idx == op.rust_idx) + .expect("wrong rust index"); let gcc_index = in_gcc_index + outputs.len(); push_to_template(modifier, gcc_index); } InlineAsmOperandRef::SymFn { .. } | InlineAsmOperandRef::SymStatic { .. } => { - let in_gcc_index = inputs.iter().position(|op| operand_idx == op.rust_idx).unwrap(); + let in_gcc_index = inputs.iter() + .position(|op| operand_idx == op.rust_idx) + .expect("wrong rust index"); let gcc_index = in_gcc_index + outputs.len(); push_to_template(None, gcc_index); } @@ -334,8 +352,10 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { InlineAsmOperandRef::InOut { reg, .. } => { let modifier = modifier_to_gcc(asm_arch, reg.reg_class(), modifier); - // The input register is tied to the input, so we can just use the index of the output register - let gcc_index = outputs.iter().position(|op| operand_idx == op.rust_idx).unwrap(); + // The input register is tied to the output, so we can just use the index of the output register + let gcc_index = outputs.iter() + .position(|op| operand_idx == op.rust_idx) + .expect("wrong rust index"); push_to_template(modifier, gcc_index); } diff --git a/src/lib.rs b/src/lib.rs index bb6bf41facd..6febccff1ff 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,7 +5,7 @@ * TODO(antoyo): remove the patches. */ -#![feature(rustc_private, decl_macro, associated_type_bounds, never_type, trusted_len, iter_intersperse)] +#![feature(rustc_private, decl_macro, associated_type_bounds, never_type, trusted_len)] #![allow(broken_intra_doc_links)] #![recursion_limit="256"] #![warn(rust_2018_idioms)] From 5af0a51dead43c6abc59fd3f528c46a153d891ed Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 28 Aug 2021 06:17:14 +0300 Subject: [PATCH 06/24] Disable LTO We don't support it yet at all --- cargo.sh | 2 +- config.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cargo.sh b/cargo.sh index a85865019c1..1001c522052 100755 --- a/cargo.sh +++ b/cargo.sh @@ -20,4 +20,4 @@ fi cmd=$1 shift -RUSTDOCFLAGS=$RUSTFLAGS cargo +${TOOLCHAIN} $cmd --target $TARGET_TRIPLE $@ +RUSTDOCFLAGS="$RUSTFLAGS" cargo +${TOOLCHAIN} $cmd --target $TARGET_TRIPLE $@ diff --git a/config.sh b/config.sh index 3ef9f11668a..203ab65a4f4 100644 --- a/config.sh +++ b/config.sh @@ -35,7 +35,7 @@ if [[ "$HOST_TRIPLE" != "$TARGET_TRIPLE" ]]; then fi fi -export RUSTFLAGS=$linker' -Cpanic=abort -Cdebuginfo=2 -Zpanic-abort-tests -Zcodegen-backend='$(pwd)'/target/'$CHANNEL'/librustc_codegen_gcc.'$dylib_ext' --sysroot '$(pwd)'/build_sysroot/sysroot' +export RUSTFLAGS=$linker' -Cpanic=abort -Cdebuginfo=2 -Clto=off -Zpanic-abort-tests -Zcodegen-backend='$(pwd)'/target/'$CHANNEL'/librustc_codegen_gcc.'$dylib_ext' --sysroot '$(pwd)'/build_sysroot/sysroot' # FIXME(antoyo): remove once the atomic shim is gone if [[ `uname` == 'Darwin' ]]; then From ac670e56a51ff598008304d8fda8d6ae04652808 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 28 Aug 2021 14:59:58 +0300 Subject: [PATCH 07/24] Handle `inout(reg) var` correctly Turns out that `+` readwrite output registers cannot be tied with input variables. --- src/asm.rs | 50 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index 1413481f504..d58e5dbd0e7 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -44,6 +44,7 @@ use crate::type_of::LayoutGccExt; // (from now on, `CC` stands for "constraint code"): // // * `out(reg_class) var` -> translated to output operand: `"=CC"(var)` +// * `inout(reg_class) var` -> translated to output operand: `"+CC"(var)` // * `in(reg_class) var` -> translated to input operand: `"CC"(var)` // // * `out(reg_class) _` -> translated to one `=r(tmp)`, where "tmp" is a temporary unused variable @@ -51,7 +52,7 @@ use crate::type_of::LayoutGccExt; // * `out("explicit register") _` -> not translated to any operands, register is simply added to clobbers list // // * `inout(reg_class) in_var => out_var` -> translated to two operands: -// output: `"+CC"(in_var)` +// output: `"=CC"(in_var)` // input: `"num"(out_var)` where num is the GCC index // of the corresponding output operand // @@ -168,7 +169,6 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { }; let tmp_var = self.current_func().new_local(None, ty, "output_register"); - outputs.push(AsmOutOperand { constraint, rust_idx, @@ -208,24 +208,38 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { // I'm not sure if GCC is so picky too, but better safe than sorry. let ty = in_value.layout.gcc_type(self.cx, false); let tmp_var = self.current_func().new_local(None, ty, "output_register"); - outputs.push(AsmOutOperand { - constraint, - rust_idx, - late, - readwrite: true, - tmp_var, - out_place, - }); - // "in" operand to be emitted later - let out_gcc_idx = outputs.len() - 1; - let constraint = Cow::Owned(out_gcc_idx.to_string()); + // If the out_place is None (i.e `inout(reg) var` syntax was used), we translate + // it to one "readwrite (+) output variable", otherwise we translate it to two + // "out and tied in" vars as described above. + if out_place.is_none() { + outputs.push(AsmOutOperand { + constraint, + rust_idx, + late, + readwrite: true, + tmp_var, + out_place, + }); + } else { + outputs.push(AsmOutOperand { + constraint, + rust_idx, + late, + readwrite: false, + tmp_var, + out_place, + }); - inputs.push(AsmInOperand { - constraint, - rust_idx, - val: in_value.immediate() - }); + let out_gcc_idx = outputs.len() - 1; + let constraint = Cow::Owned(out_gcc_idx.to_string()); + + inputs.push(AsmInOperand { + constraint, + rust_idx, + val: in_value.immediate() + }); + } } InlineAsmOperandRef::Const { ref string } => { From 2b9a99131a084396e233b7b044260ffcac37b8c3 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 28 Aug 2021 15:08:30 +0300 Subject: [PATCH 08/24] Add limited support for llvm_asm! --- src/asm.rs | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index d58e5dbd0e7..2b246bccc07 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -97,17 +97,30 @@ impl AsmOutOperand<'_, '_, '_> { impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { - fn codegen_llvm_inline_asm(&mut self, _ia: &LlvmInlineAsmInner, _outputs: Vec>>, mut _inputs: Vec>, span: Span) -> bool { - // TODO(@Commeownist): switch to `struct_span_err_with_code` - // once we get this merged into rustc - self.sess().struct_span_err(span, "GCC backend does not support `llvm_asm!`") - .help("consider using the `asm!` macro instead") - .emit(); - - // We return `true` even though we've failed to generate the asm + fn codegen_llvm_inline_asm(&mut self, ia: &LlvmInlineAsmInner, outputs: Vec>>, inputs: Vec>, span: Span) -> bool { + if ia.asm.as_str().is_empty() && outputs.is_empty() { + // TODO(@Commeownist): there's one use of `llvm_asm` in rustc sysroot we can't get rid of just yet. + // Fortunately, it's used as a simple black box to make sure that inputs are not optimized away. + // Let's just emulate it. + let block = self.llbb(); + let extended_asm = block.add_extended_asm(None, ""); + for input in inputs { + extended_asm.add_input_operand(None, "r", input); + } + extended_asm.add_clobber("memory"); + extended_asm.set_volatile_flag(true); + } else { + // TODO(@Commeownist): switch to `struct_span_err_with_code` + // once we get this merged into rustc + self.sess().struct_span_err(span, "GCC backend does not support `llvm_asm!`") + .help("consider using the `asm!` macro instead") + .emit(); + } + + // We return `true` even if we've failed to generate the asm // because we want to suppress the "malformed inline assembly" error // generated by the frontend. - return true; + true } fn codegen_inline_asm(&mut self, template: &[InlineAsmTemplatePiece], rust_operands: &[InlineAsmOperandRef<'tcx, Self>], options: InlineAsmOptions, span: &[Span]) { From aeda5d629a3e86abd1bf0e223f7e0d9a6a777f4d Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 28 Aug 2021 17:45:23 +0300 Subject: [PATCH 09/24] Handle CHANNEL correctly --- config.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.sh b/config.sh index 203ab65a4f4..98caeb7407e 100644 --- a/config.sh +++ b/config.sh @@ -35,7 +35,7 @@ if [[ "$HOST_TRIPLE" != "$TARGET_TRIPLE" ]]; then fi fi -export RUSTFLAGS=$linker' -Cpanic=abort -Cdebuginfo=2 -Clto=off -Zpanic-abort-tests -Zcodegen-backend='$(pwd)'/target/'$CHANNEL'/librustc_codegen_gcc.'$dylib_ext' --sysroot '$(pwd)'/build_sysroot/sysroot' +export RUSTFLAGS="$linker -Cpanic=abort -Cdebuginfo=2 -Clto=off -Zpanic-abort-tests -Zcodegen-backend=$(pwd)/target/${CHANNEL:-debug}/librustc_codegen_gcc.$dylib_ext --sysroot $(pwd)/build_sysroot/sysroot" # FIXME(antoyo): remove once the atomic shim is gone if [[ `uname` == 'Darwin' ]]; then From 8596d50e41f1f93e1eff82ee1fd84ea7602dc778 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Mon, 30 Aug 2021 15:50:45 +0300 Subject: [PATCH 10/24] Add support for arbitrary explicit registers --- Cargo.lock | 4 +-- src/asm.rs | 78 +++++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 65 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 952c03b05c7..f3e07fd08ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -56,7 +56,7 @@ dependencies = [ [[package]] name = "gccjit" version = "1.0.0" -source = "git+https://github.com/antoyo/gccjit.rs#0572117c7ffdfcb0e6c6526d45266c3f34796bea" +source = "git+https://github.com/antoyo/gccjit.rs#54be27e41fff7b6ab532e2e21a82df50a12b9ad3" dependencies = [ "gccjit_sys", ] @@ -64,7 +64,7 @@ dependencies = [ [[package]] name = "gccjit_sys" version = "0.0.1" -source = "git+https://github.com/antoyo/gccjit.rs#0572117c7ffdfcb0e6c6526d45266c3f34796bea" +source = "git+https://github.com/antoyo/gccjit.rs#54be27e41fff7b6ab532e2e21a82df50a12b9ad3" dependencies = [ "libc 0.1.12", ] diff --git a/src/asm.rs b/src/asm.rs index 2b246bccc07..a6bd139b197 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -123,7 +123,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { true } - fn codegen_inline_asm(&mut self, template: &[InlineAsmTemplatePiece], rust_operands: &[InlineAsmOperandRef<'tcx, Self>], options: InlineAsmOptions, span: &[Span]) { + fn codegen_inline_asm(&mut self, template: &[InlineAsmTemplatePiece], rust_operands: &[InlineAsmOperandRef<'tcx, Self>], options: InlineAsmOptions, _span: &[Span]) { let asm_arch = self.tcx.sess.asm_arch.unwrap(); let att_dialect = matches!(asm_arch, InlineAsmArch::X86 | InlineAsmArch::X86_64) && options.contains(InlineAsmOptions::ATT_SYNTAX); @@ -278,37 +278,83 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } for (rust_idx, op) in rust_operands.iter().enumerate() { - let not_yet = |reg_name| { - // TODO(@Commeownist) - let err = format!("GCC backend does not support explicit registers such as `\"{}\"` just yet", reg_name); - self.sess().struct_span_err(span[rust_idx], &err) - .note("need register variables support in libgccjit") - .emit(); - }; - match *op { // `out("explicit register") _` - InlineAsmOperandRef::Out { reg, late:_, place:_ } => { + InlineAsmOperandRef::Out { reg, late, place } => { if let Err(reg_name) = reg_to_gcc(reg) { - not_yet(reg_name); + let out_place = if let Some(place) = place { + place + } else { + // processed in the previous pass + continue + }; + + let ty = out_place.layout.gcc_type(self.cx, false); + let tmp_var = self.current_func().new_local(None, ty, "output_register"); + tmp_var.set_register_name(reg_name); + + outputs.push(AsmOutOperand { + constraint: "r".into(), + rust_idx, + late, + readwrite: false, + tmp_var, + out_place: Some(out_place) + }); } // processed in the previous pass } // `in("explicit register") var` - InlineAsmOperandRef::In { reg, value:_ } => { + InlineAsmOperandRef::In { reg, value } => { if let Err(reg_name) = reg_to_gcc(reg) { - not_yet(reg_name); + let ty = value.layout.gcc_type(self.cx, false); + let reg_var = self.current_func().new_local(None, ty, "input_register"); + reg_var.set_register_name(reg_name); + // TODO(@Commeownist): Should use `OperandValue::store` instead? + self.llbb().add_assignment(None, reg_var, value.immediate()); + + inputs.push(AsmInOperand { + constraint: "r".into(), + rust_idx, + val: reg_var.to_rvalue() + }); } // processed in the previous pass } // `inout("explicit register") in_var => out_var/_` - InlineAsmOperandRef::InOut { reg, late:_, in_value:_, out_place:_ } => { + InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => { if let Err(reg_name) = reg_to_gcc(reg) { - not_yet(reg_name); + let out_place = if let Some(place) = out_place { + place + } else { + // processed in the previous pass + continue + }; + + // See explanation in the first pass. + let ty = in_value.layout.gcc_type(self.cx, false); + let tmp_var = self.current_func().new_local(None, ty, "output_register"); + tmp_var.set_register_name(reg_name); + + outputs.push(AsmOutOperand { + constraint: "r".into(), + rust_idx, + late, + readwrite: false, + tmp_var, + out_place: Some(out_place) + }); + + let constraint = Cow::Owned((outputs.len() - 1).to_string()); + inputs.push(AsmInOperand { + constraint, + rust_idx, + val: in_value.immediate() + }); } // processed in the previous pass @@ -443,6 +489,8 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { if let Some(place) = op.out_place { OperandValue::Immediate(op.tmp_var.to_rvalue()).store(self, place); } + // TODO:(@Commeownist): if out_place is None, maybe we should use `Block::add_eval` + // to discard the unused `tmp_var`? The rest of backend seems to do that. } // TODO(@Commeownist): maybe we need to follow up with a call to `__builtin_unreachable` From 29c3d2d991662974f2cbcfa25f882943c3931b91 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Tue, 31 Aug 2021 03:26:18 +0300 Subject: [PATCH 11/24] Handle symbols properly --- src/asm.rs | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index a6bd139b197..d95baf04a75 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -2,10 +2,10 @@ use gccjit::{LValue, RValue, ToRValue, Type}; use rustc_ast::ast::{InlineAsmOptions, InlineAsmTemplatePiece}; use rustc_codegen_ssa::mir::operand::OperandValue; use rustc_codegen_ssa::mir::place::PlaceRef; -use rustc_codegen_ssa::traits::{AsmBuilderMethods, AsmMethods, BaseTypeMethods, BuilderMethods, GlobalAsmOperandRef, InlineAsmOperandRef, MiscMethods}; +use rustc_codegen_ssa::traits::{AsmBuilderMethods, AsmMethods, BaseTypeMethods, BuilderMethods, GlobalAsmOperandRef, InlineAsmOperandRef}; use rustc_hir::LlvmInlineAsmInner; -use rustc_middle::bug; +use rustc_middle::{bug, ty::Instance}; use rustc_span::Span; use rustc_target::asm::*; @@ -260,19 +260,10 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } InlineAsmOperandRef::SymFn { instance } => { - inputs.push(AsmInOperand { - constraint: Cow::Borrowed("s"), - rust_idx, - val: self.cx.get_fn(instance) - }); + constants_len += self.tcx.symbol_name(instance).name.len(); } - InlineAsmOperandRef::SymStatic { def_id } => { - inputs.push(AsmInOperand { - constraint: Cow::Borrowed("s"), - rust_idx, - val: self.cx.get_static(def_id) - }); + constants_len += self.tcx.symbol_name(Instance::mono(self.tcx, def_id)).name.len(); } } } @@ -413,15 +404,6 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { push_to_template(modifier, gcc_index); } - InlineAsmOperandRef::SymFn { .. } - | InlineAsmOperandRef::SymStatic { .. } => { - let in_gcc_index = inputs.iter() - .position(|op| operand_idx == op.rust_idx) - .expect("wrong rust index"); - let gcc_index = in_gcc_index + outputs.len(); - push_to_template(None, gcc_index); - } - InlineAsmOperandRef::InOut { reg, .. } => { let modifier = modifier_to_gcc(asm_arch, reg.reg_class(), modifier); @@ -432,6 +414,19 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { push_to_template(modifier, gcc_index); } + InlineAsmOperandRef::SymFn { instance } => { + let name = self.tcx.symbol_name(instance).name; + template_str.push_str(name); + } + + InlineAsmOperandRef::SymStatic { def_id } => { + // TODO(@Commeownist): This may not be sufficient for all kinds of statics. + // Some statics may need the `@plt` suffix, like thread-local vars. + let instance = Instance::mono(self.tcx, def_id); + let name = self.tcx.symbol_name(instance).name; + template_str.push_str(name); + } + InlineAsmOperandRef::Const { ref string } => { // Const operands get injected directly into the template if att_dialect { From e435e57eaaa1f66c6613a5269b696bf52546fdda Mon Sep 17 00:00:00 2001 From: Commeownist Date: Tue, 31 Aug 2021 03:26:58 +0300 Subject: [PATCH 12/24] Add rudimentary asm tests --- tests/run/asm.rs | 87 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/tests/run/asm.rs b/tests/run/asm.rs index bd76153e047..9c0055b0b6b 100644 --- a/tests/run/asm.rs +++ b/tests/run/asm.rs @@ -62,5 +62,92 @@ fn main() { } assert_eq!(x, 43); + // check inout(reg_class) x + let mut x: u64 = 42; + unsafe { + asm!("add {0}, {0}", + inout(reg) x + ); + } + assert_eq!(x, 84); + + // check inout("reg") x + let mut x: u64 = 42; + unsafe { + asm!("add r11, r11", + inout("r11") x + ); + } + assert_eq!(x, 84); + + // check a mix of + // in("reg") + // inout(class) x => y + // inout (class) x + let x: u64 = 702; + let y: u64 = 100; + let res: u64; + let mut rem: u64 = 0; + unsafe { + asm!("div r11", + in("r11") y, + inout("eax") x => res, + inout("edx") rem, + ); + } + assert_eq!(res, 7); + assert_eq!(rem, 2); + + // check const + let mut x: u64 = 42; + unsafe { + asm!("add {}, {}", + inout(reg) x, + const 1 + ); + } + assert_eq!(x, 43); + + // check const (ATT syntax) + let mut x: u64 = 42; + unsafe { + asm!("add {}, {}", + const 1, + inout(reg) x, + options(att_syntax) + ); + } + assert_eq!(x, 43); + + // check sym fn + extern "C" fn foo() -> u64 { 42 } + let x: u64; + unsafe { + asm!("call {}", sym foo, lateout("rax") x); + } + assert_eq!(x, 42); + + // check sym fn (ATT syntax) + let x: u64; + unsafe { + asm!("call {}", sym foo, lateout("rax") x, options(att_syntax)); + } + assert_eq!(x, 42); + + // check sym static + static FOO: u64 = 42; + let x: u64; + unsafe { + asm!("mov {1}, qword ptr [rip + {0}]", sym FOO, lateout(reg) x); + } + assert_eq!(x, 42); + + // check sym static (ATT syntax) + let x: u64; + unsafe { + asm!("movq {0}(%rip), {1}", sym FOO, lateout(reg) x, options(att_syntax)); + } + assert_eq!(x, 42); + assert_eq!(unsafe { add_asm(40, 2) }, 42); } From ae0048193d91eb31baf785a4ba4cb8e66fd8e8db Mon Sep 17 00:00:00 2001 From: Commeownist Date: Tue, 31 Aug 2021 03:27:38 +0300 Subject: [PATCH 13/24] Improve template len estimation --- src/asm.rs | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index d95baf04a75..30251a8547f 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -64,6 +64,10 @@ use crate::type_of::LayoutGccExt; // and one register variable assigned to the desired register. // +const ATT_SYNTAX_INS: &str = ".att_syntax noprefix\n\t"; +const INTEL_SYNTAX_INS: &str = "\n\t.intel_syntax noprefix"; + + struct AsmOutOperand<'a, 'tcx, 'gcc> { rust_idx: usize, constraint: &'a str, @@ -360,7 +364,11 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } // Build the template string - let mut template_str = String::with_capacity(estimate_template_length(template, constants_len)); + let mut template_str = String::with_capacity(estimate_template_length(template, constants_len, att_dialect)); + if !intel_dialect { + template_str.push_str(ATT_SYNTAX_INS); + } + for piece in template { match *piece { InlineAsmTemplatePiece::String(ref string) => { @@ -439,16 +447,10 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } } - let template_str = - if intel_dialect { - template_str - } - else { - // FIXME(antoyo): this might break the "m" memory constraint: - // https://stackoverflow.com/a/9347957/389119 - format!(".att_syntax noprefix\n\t{}\n\t.intel_syntax noprefix", template_str) - }; - + if !intel_dialect { + template_str.push_str(INTEL_SYNTAX_INS); + } + let block = self.llbb(); let extended_asm = block.add_extended_asm(None, &template_str); @@ -496,15 +498,15 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } } -fn estimate_template_length(template: &[InlineAsmTemplatePiece], constants_len: usize) -> usize { +fn estimate_template_length(template: &[InlineAsmTemplatePiece], constants_len: usize, att_dialect: bool) -> usize { let len: usize = template.iter().map(|piece| { match *piece { InlineAsmTemplatePiece::String(ref string) => { string.len() } InlineAsmTemplatePiece::Placeholder { .. } => { - // '%' + 1 char modifier + 2 chars index - 4 + // '%' + 1 char modifier + 1 char index + 3 } } }) @@ -513,7 +515,12 @@ fn estimate_template_length(template: &[InlineAsmTemplatePiece], constants_len: // increase it by 5% to account for possible '%' signs that'll be duplicated // I pulled the number out of blue, but should be fair enough // as the upper bound - (len as f32 * 1.05) as usize + constants_len + let mut res = (len as f32 * 1.05) as usize + constants_len; + + if att_dialect { + res += INTEL_SYNTAX_INS.len() + ATT_SYNTAX_INS.len(); + } + res } /// Converts a register class to a GCC constraint code. From a7804e361b63e548c5a126bcc1218b9489a916e7 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Wed, 1 Sep 2021 20:52:13 +0300 Subject: [PATCH 14/24] Exclude llvm_asm! tests from tests runs --- test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test.sh b/test.sh index c246d8eacc5..4550e591f60 100755 --- a/test.sh +++ b/test.sh @@ -156,7 +156,7 @@ EOF rustc -V | cut -d' ' -f3 | tr -d '(' git checkout $(rustc -V | cut -d' ' -f3 | tr -d '(') src/test -for test in $(rg -i --files-with-matches "//(\[\w+\])?~|// error-pattern:|// build-fail|// run-fail|-Cllvm-args" src/test/ui); do +for test in $(rg -i --files-with-matches "//(\[\w+\])?~|// error-pattern:|// build-fail|// run-fail|-Cllvm-args|llvm_asm!\\(\".+\"\\)" src/test/ui); do rm $test done From 754bbf934597021c35713e77a7ee29ced5e95385 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Thu, 2 Sep 2021 19:15:03 +0300 Subject: [PATCH 15/24] Address review requests --- src/asm.rs | 82 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index 30251a8547f..e4d22940943 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -37,8 +37,9 @@ use crate::type_of::LayoutGccExt; // // 4. Furthermore, GCC Extended Asm does not support explicit register constraints // (like `out("eax")`) directly, offering so-called "local register variables" -// as a workaround. This variables need to be declared and initialized *before* -// the Extended Asm block but *after* normal local variables. +// as a workaround. These variables need to be declared and initialized *before* +// the Extended Asm block but *after* normal local variables +// (see comment in `codegen_inline_asm` for explanation). // // With that in mind, let's see how we translate Rust syntax to GCC // (from now on, `CC` stands for "constraint code"): @@ -99,6 +100,11 @@ impl AsmOutOperand<'_, '_, '_> { } } +enum ConstraintOrRegister<'a> { + Constraint(&'a str), + Register(&'a str) +} + impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { fn codegen_llvm_inline_asm(&mut self, ia: &LlvmInlineAsmInner, outputs: Vec>>, inputs: Vec>, span: Span) -> bool { @@ -129,10 +135,9 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { fn codegen_inline_asm(&mut self, template: &[InlineAsmTemplatePiece], rust_operands: &[InlineAsmOperandRef<'tcx, Self>], options: InlineAsmOptions, _span: &[Span]) { let asm_arch = self.tcx.sess.asm_arch.unwrap(); - let att_dialect = matches!(asm_arch, InlineAsmArch::X86 | InlineAsmArch::X86_64) && - options.contains(InlineAsmOptions::ATT_SYNTAX); - let intel_dialect = matches!(asm_arch, InlineAsmArch::X86 | InlineAsmArch::X86_64) && - !options.contains(InlineAsmOptions::ATT_SYNTAX); + let is_x86 = matches!(asm_arch, InlineAsmArch::X86 | InlineAsmArch::X86_64); + let att_dialect = is_x86 && options.contains(InlineAsmOptions::ATT_SYNTAX); + let intel_dialect = is_x86 && !options.contains(InlineAsmOptions::ATT_SYNTAX); // GCC index of an output operand equals its position in the array let mut outputs = vec![]; @@ -165,21 +170,24 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { // Also, we don't emit any asm operands immediately; we save them to // the one of the buffers to be emitted later. + // 1. Normal variables (and saving operands to buffers). for (rust_idx, op) in rust_operands.iter().enumerate() { match *op { InlineAsmOperandRef::Out { reg, late, place } => { + use ConstraintOrRegister::*; + let (constraint, ty) = match (reg_to_gcc(reg), place) { - (Ok(constraint), Some(place)) => (constraint, place.layout.gcc_type(self.cx, false)), + (Constraint(constraint), Some(place)) => (constraint, place.layout.gcc_type(self.cx, false)), // When `reg` is a class and not an explicit register but the out place is not specified, // we need to create an unused output variable to assign the output to. This var // needs to be of a type that's "compatible" with the register class, but specific type // doesn't matter. - (Ok(constraint), None) => (constraint, dummy_output_type(self.cx, reg.reg_class())), - (Err(_), Some(_)) => { + (Constraint(constraint), None) => (constraint, dummy_output_type(self.cx, reg.reg_class())), + (Register(_), Some(_)) => { // left for the next pass continue }, - (Err(reg_name), None) => { + (Register(reg_name), None) => { clobbers.push(reg_name); continue } @@ -197,9 +205,9 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } InlineAsmOperandRef::In { reg, value } => { - if let Ok(constraint) = reg_to_gcc(reg).map(Cow::Borrowed) { + if let ConstraintOrRegister::Constraint(constraint) = reg_to_gcc(reg) { inputs.push(AsmInOperand { - constraint, + constraint: Cow::Borrowed(constraint), rust_idx, val: value.immediate() }); @@ -210,7 +218,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => { - let constraint = if let Ok(constraint) = reg_to_gcc(reg) { + let constraint = if let ConstraintOrRegister::Constraint(constraint) = reg_to_gcc(reg) { constraint } else { // left for the next pass @@ -229,25 +237,17 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { // If the out_place is None (i.e `inout(reg) var` syntax was used), we translate // it to one "readwrite (+) output variable", otherwise we translate it to two // "out and tied in" vars as described above. - if out_place.is_none() { - outputs.push(AsmOutOperand { - constraint, - rust_idx, - late, - readwrite: true, - tmp_var, - out_place, - }); - } else { - outputs.push(AsmOutOperand { - constraint, - rust_idx, - late, - readwrite: false, - tmp_var, - out_place, - }); + let readwrite = out_place.is_none(); + outputs.push(AsmOutOperand { + constraint, + rust_idx, + late, + readwrite: false, + tmp_var, + out_place, + }); + if !readwrite { let out_gcc_idx = outputs.len() - 1; let constraint = Cow::Owned(out_gcc_idx.to_string()); @@ -272,11 +272,12 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } } + // 2. Register variables. for (rust_idx, op) in rust_operands.iter().enumerate() { match *op { // `out("explicit register") _` InlineAsmOperandRef::Out { reg, late, place } => { - if let Err(reg_name) = reg_to_gcc(reg) { + if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) { let out_place = if let Some(place) = place { place } else { @@ -303,7 +304,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { // `in("explicit register") var` InlineAsmOperandRef::In { reg, value } => { - if let Err(reg_name) = reg_to_gcc(reg) { + if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) { let ty = value.layout.gcc_type(self.cx, false); let reg_var = self.current_func().new_local(None, ty, "input_register"); reg_var.set_register_name(reg_name); @@ -322,7 +323,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { // `inout("explicit register") in_var => out_var/_` InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => { - if let Err(reg_name) = reg_to_gcc(reg) { + if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) { let out_place = if let Some(place) = out_place { place } else { @@ -363,7 +364,8 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } } - // Build the template string + // 3. Build the template string + let mut template_str = String::with_capacity(estimate_template_length(template, constants_len, att_dialect)); if !intel_dialect { template_str.push_str(ATT_SYNTAX_INS); @@ -450,7 +452,9 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { if !intel_dialect { template_str.push_str(INTEL_SYNTAX_INS); } - + + // 4. Generate Extended Asm block + let block = self.llbb(); let extended_asm = block.add_extended_asm(None, &template_str); @@ -524,7 +528,7 @@ fn estimate_template_length(template: &[InlineAsmTemplatePiece], constants_len: } /// Converts a register class to a GCC constraint code. -fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> Result<&'static str, &'static str> { +fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> ConstraintOrRegister<'static> { let constraint = match reg { // For vector registers LLVM wants the register name to match the type size. InlineAsmRegOrRegClass::Reg(reg) => { @@ -537,7 +541,7 @@ fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> Result<&'static str, &'static str> "si" => "S", "di" => "D", // For registers like r11, we have to create a register variable: https://stackoverflow.com/a/31774784/389119 - name => return Err(name), + name => return ConstraintOrRegister::Register(name), } }, InlineAsmRegOrRegClass::RegClass(reg) => match reg { @@ -586,7 +590,7 @@ fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> Result<&'static str, &'static str> } }; - Ok(constraint) + ConstraintOrRegister::Constraint(constraint) } /// Type to use for outputs that are discarded. It doesn't really matter what From 1e5e302cceb45684cecdeb23eb497b5887717894 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Thu, 2 Sep 2021 19:15:53 +0300 Subject: [PATCH 16/24] Use add_eval to discard unused temporary variables --- src/asm.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index e4d22940943..1c80725412e 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -485,13 +485,21 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { // TODO(@Commeownist): figure out how to align stack } - // Write results to outputs + // Write results to outputs. + // + // We need to do this because: + // 1. Turning `PlaceRef` into `RValue` is error-prone and has nasty edge cases + // (especially with current `rustc_backend_ssa` API). + // 2. Not every output operand has an `out_place`, and it's required by `add_output_operand`. + // + // Instead, we generate a temporary output variable for each output operand, and then this loop, + // generates `out_place = tmp_var;` assignments if out_place exists, or `(void)tmp_var;` if it doesn't. for op in &outputs { if let Some(place) = op.out_place { OperandValue::Immediate(op.tmp_var.to_rvalue()).store(self, place); + } else { + block.add_eval(None, op.tmp_var.to_rvalue()); } - // TODO:(@Commeownist): if out_place is None, maybe we should use `Block::add_eval` - // to discard the unused `tmp_var`? The rest of backend seems to do that. } // TODO(@Commeownist): maybe we need to follow up with a call to `__builtin_unreachable` From 4a19b459d5bb16a8cdda91f5c57d9a821ba4ee32 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Thu, 2 Sep 2021 20:00:39 +0300 Subject: [PATCH 17/24] Fix copy-paste bug --- src/asm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/asm.rs b/src/asm.rs index 1c80725412e..ac408686d33 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -242,7 +242,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { constraint, rust_idx, late, - readwrite: false, + readwrite, tmp_var, out_place, }); From 60ec630ff0089dd6d83c14b0b99829cb566d7041 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 4 Sep 2021 01:23:58 +0300 Subject: [PATCH 18/24] Remove add_eval --- src/asm.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index ac408686d33..eea64f25699 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -493,20 +493,13 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { // 2. Not every output operand has an `out_place`, and it's required by `add_output_operand`. // // Instead, we generate a temporary output variable for each output operand, and then this loop, - // generates `out_place = tmp_var;` assignments if out_place exists, or `(void)tmp_var;` if it doesn't. + // generates `out_place = tmp_var;` assignments if out_place exists. for op in &outputs { if let Some(place) = op.out_place { OperandValue::Immediate(op.tmp_var.to_rvalue()).store(self, place); - } else { - block.add_eval(None, op.tmp_var.to_rvalue()); } } - // TODO(@Commeownist): maybe we need to follow up with a call to `__builtin_unreachable` - // if NORETURN option is set. - // - // In fact, GCC docs for __builtin_unreachable do describe it as a good fit after diverging asm blocks! - // https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable } } From 29c212a432d7be4bcf6432eb66c009f40c9b4306 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 4 Sep 2021 01:24:34 +0300 Subject: [PATCH 19/24] Insert `__builtin_unreachable()` after diverging asm blocks --- src/asm.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/asm.rs b/src/asm.rs index eea64f25699..952c1eb0954 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -484,6 +484,11 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { if !options.contains(InlineAsmOptions::NOSTACK) { // TODO(@Commeownist): figure out how to align stack } + if options.contains(InlineAsmOptions::NORETURN) { + let builtin_unreachable = self.context.get_builtin_function("__builtin_unreachable"); + let builtin_unreachable: RValue<'gcc> = unsafe { std::mem::transmute(builtin_unreachable) }; + self.call(self.type_void(), builtin_unreachable, &[], None); + } // Write results to outputs. // From 75d4f6ffae756cf2ff57f026ded0918c0cfcf84f Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 4 Sep 2021 01:26:01 +0300 Subject: [PATCH 20/24] Fix code style --- src/asm.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index 952c1eb0954..2c329e7dd63 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -119,7 +119,8 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { } extended_asm.add_clobber("memory"); extended_asm.set_volatile_flag(true); - } else { + } + else { // TODO(@Commeownist): switch to `struct_span_err_with_code` // once we get this merged into rustc self.sess().struct_span_err(span, "GCC backend does not support `llvm_asm!`") @@ -211,7 +212,8 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { rust_idx, val: value.immediate() }); - } else { + } + else { // left for the next pass continue } @@ -220,7 +222,8 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => { let constraint = if let ConstraintOrRegister::Constraint(constraint) = reg_to_gcc(reg) { constraint - } else { + } + else { // left for the next pass continue }; @@ -280,7 +283,8 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) { let out_place = if let Some(place) = place { place - } else { + } + else { // processed in the previous pass continue }; @@ -326,7 +330,8 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) { let out_place = if let Some(place) = out_place { place - } else { + } + else { // processed in the previous pass continue }; From 3dc092149d06fcf289c19055777163204731adc1 Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 4 Sep 2021 02:31:18 +0300 Subject: [PATCH 21/24] Address review comments --- src/asm.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index 2c329e7dd63..624d9a1ed62 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -100,9 +100,9 @@ impl AsmOutOperand<'_, '_, '_> { } } -enum ConstraintOrRegister<'a> { - Constraint(&'a str), - Register(&'a str) +enum ConstraintOrRegister { + Constraint(&'static str), + Register(&'static str) } @@ -312,7 +312,6 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { let ty = value.layout.gcc_type(self.cx, false); let reg_var = self.current_func().new_local(None, ty, "input_register"); reg_var.set_register_name(reg_name); - // TODO(@Commeownist): Should use `OperandValue::store` instead? self.llbb().add_assignment(None, reg_var, value.immediate()); inputs.push(AsmInOperand { @@ -539,7 +538,7 @@ fn estimate_template_length(template: &[InlineAsmTemplatePiece], constants_len: } /// Converts a register class to a GCC constraint code. -fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> ConstraintOrRegister<'static> { +fn reg_to_gcc(reg: InlineAsmRegOrRegClass) -> ConstraintOrRegister { let constraint = match reg { // For vector registers LLVM wants the register name to match the type size. InlineAsmRegOrRegClass::Reg(reg) => { From 5266aa1d02e5434247a14e054cd663a25db53d1c Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 4 Sep 2021 03:49:53 +0300 Subject: [PATCH 22/24] Revert "Exclude llvm_asm! tests from tests runs" This reverts commit a7804e361b63e548c5a126bcc1218b9489a916e7. --- test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test.sh b/test.sh index 4550e591f60..c246d8eacc5 100755 --- a/test.sh +++ b/test.sh @@ -156,7 +156,7 @@ EOF rustc -V | cut -d' ' -f3 | tr -d '(' git checkout $(rustc -V | cut -d' ' -f3 | tr -d '(') src/test -for test in $(rg -i --files-with-matches "//(\[\w+\])?~|// error-pattern:|// build-fail|// run-fail|-Cllvm-args|llvm_asm!\\(\".+\"\\)" src/test/ui); do +for test in $(rg -i --files-with-matches "//(\[\w+\])?~|// error-pattern:|// build-fail|// run-fail|-Cllvm-args" src/test/ui); do rm $test done From 00c26df0ddac8cadd105206e3f9323f45283fbdd Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 4 Sep 2021 03:52:47 +0300 Subject: [PATCH 23/24] Exclude all llvm_asm! tests --- test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test.sh b/test.sh index c246d8eacc5..114db7d53f5 100755 --- a/test.sh +++ b/test.sh @@ -162,7 +162,7 @@ done git checkout -- src/test/ui/issues/auxiliary/issue-3136-a.rs # contains //~ERROR, but shouldn't be removed -rm -r src/test/ui/{abi*,extern/,panic-runtime/,panics/,unsized-locals/,proc-macro/,threads-sendsync/,thinlto/,simd*,borrowck/,test*,*lto*.rs} || true +rm -r src/test/ui/{abi*,extern/,llvm-asm/,panic-runtime/,panics/,unsized-locals/,proc-macro/,threads-sendsync/,thinlto/,simd*,borrowck/,test*,*lto*.rs} || true for test in $(rg --files-with-matches "catch_unwind|should_panic|thread|lto" src/test/ui); do rm $test done From 97395243e14d32e9421f74451af018f6dfee987b Mon Sep 17 00:00:00 2001 From: Commeownist Date: Sat, 4 Sep 2021 19:02:35 +0300 Subject: [PATCH 24/24] Address review --- src/asm.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/asm.rs b/src/asm.rs index 624d9a1ed62..512b9f553f5 100644 --- a/src/asm.rs +++ b/src/asm.rs @@ -155,7 +155,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { // There are rules we must adhere to if we want GCC to do the right thing: // - // * Every local variable the asm block uses as output must be declared *before* + // * Every local variable that the asm block uses as an output must be declared *before* // the asm block. // * There must be no instructions whatsoever between the register variables and the asm. // @@ -237,7 +237,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { let ty = in_value.layout.gcc_type(self.cx, false); let tmp_var = self.current_func().new_local(None, ty, "output_register"); - // If the out_place is None (i.e `inout(reg) var` syntax was used), we translate + // If the out_place is None (i.e `inout(reg) _` syntax was used), we translate // it to one "readwrite (+) output variable", otherwise we translate it to two // "out and tied in" vars as described above. let readwrite = out_place.is_none(); @@ -278,7 +278,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { // 2. Register variables. for (rust_idx, op) in rust_operands.iter().enumerate() { match *op { - // `out("explicit register") _` + // `out("explicit register") var` InlineAsmOperandRef::Out { reg, late, place } => { if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) { let out_place = if let Some(place) = place { @@ -324,7 +324,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { // processed in the previous pass } - // `inout("explicit register") in_var => out_var/_` + // `inout("explicit register") in_var => out_var` InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => { if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) { let out_place = if let Some(place) = out_place { @@ -363,7 +363,7 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> { InlineAsmOperandRef::Const { .. } | InlineAsmOperandRef::SymFn { .. } | InlineAsmOperandRef::SymStatic { .. } => { - // processed on the previous pass + // processed in the previous pass } } }