Skip to content

Commit

Permalink
Auto merge of rust-lang#130506 - nnethercote:rustc_codegen_llvm-clean…
Browse files Browse the repository at this point in the history
…ups, r=jieyouxu

`rustc_codegen_llvm` cleanups

Some improvements I found while reading through this crate's code.

r? `@michaelwoerister`
  • Loading branch information
bors committed Sep 20, 2024
2 parents 2b11f26 + 1f35940 commit 1a5a224
Show file tree
Hide file tree
Showing 21 changed files with 515 additions and 648 deletions.
32 changes: 16 additions & 16 deletions compiler/rustc_codegen_gcc/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,22 +80,14 @@ impl<'gcc, 'tcx> ConstCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
self.const_undef(typ)
}

fn const_int(&self, typ: Type<'gcc>, int: i64) -> RValue<'gcc> {
self.gcc_int(typ, int)
}

fn const_uint(&self, typ: Type<'gcc>, int: u64) -> RValue<'gcc> {
self.gcc_uint(typ, int)
}

fn const_uint_big(&self, typ: Type<'gcc>, num: u128) -> RValue<'gcc> {
self.gcc_uint_big(typ, num)
}

fn const_bool(&self, val: bool) -> RValue<'gcc> {
self.const_uint(self.type_i1(), val as u64)
}

fn const_i8(&self, i: i8) -> RValue<'gcc> {
self.const_int(self.type_i8(), i as i64)
}

fn const_i16(&self, i: i16) -> RValue<'gcc> {
self.const_int(self.type_i16(), i as i64)
}
Expand All @@ -104,8 +96,12 @@ impl<'gcc, 'tcx> ConstCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
self.const_int(self.type_i32(), i as i64)
}

fn const_i8(&self, i: i8) -> RValue<'gcc> {
self.const_int(self.type_i8(), i as i64)
fn const_int(&self, typ: Type<'gcc>, int: i64) -> RValue<'gcc> {
self.gcc_int(typ, int)
}

fn const_u8(&self, i: u8) -> RValue<'gcc> {
self.const_uint(self.type_u8(), i as u64)
}

fn const_u32(&self, i: u32) -> RValue<'gcc> {
Expand All @@ -130,8 +126,12 @@ impl<'gcc, 'tcx> ConstCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
self.const_uint(self.usize_type, i)
}

fn const_u8(&self, i: u8) -> RValue<'gcc> {
self.const_uint(self.type_u8(), i as u64)
fn const_uint(&self, typ: Type<'gcc>, int: u64) -> RValue<'gcc> {
self.gcc_uint(typ, int)
}

fn const_uint_big(&self, typ: Type<'gcc>, num: u128) -> RValue<'gcc> {
self.gcc_uint_big(typ, num)
}

fn const_real(&self, typ: Type<'gcc>, val: f64) -> RValue<'gcc> {
Expand Down
543 changes: 251 additions & 292 deletions compiler/rustc_codegen_llvm/src/asm.rs

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,9 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) {
to_add.push(AttributeKind::Naked.create_attr(cx.llcx));
// HACK(jubilee): "indirect branch tracking" works by attaching prologues to functions.
// And it is a module-level attribute, so the alternative is pulling naked functions into new LLVM modules.
// Otherwise LLVM's "naked" functions come with endbr prefixes per https://github.com/rust-lang/rust/issues/98768
// And it is a module-level attribute, so the alternative is pulling naked functions into
// new LLVM modules. Otherwise LLVM's "naked" functions come with endbr prefixes per
// https://github.com/rust-lang/rust/issues/98768
to_add.push(AttributeKind::NoCfCheck.create_attr(cx.llcx));
if llvm_util::get_version() < (19, 0, 0) {
// Prior to LLVM 19, branch-target-enforcement was disabled by setting the attribute to
Expand Down Expand Up @@ -454,7 +455,8 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
flags |= AllocKindFlags::Zeroed;
}
to_add.push(llvm::CreateAllocKindAttr(cx.llcx, flags));
// apply to return place instead of function (unlike all other attributes applied in this function)
// apply to return place instead of function (unlike all other attributes applied in this
// function)
let no_alias = AttributeKind::NoAlias.create_attr(cx.llcx);
attributes::apply_to_llfn(llfn, AttributePlace::ReturnValue, &[no_alias]);
}
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_codegen_llvm/src/back/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,15 @@ fn get_bitcode_slice_from_object_data<'a>(
obj: &'a [u8],
cgcx: &CodegenContext<LlvmCodegenBackend>,
) -> Result<&'a [u8], LtoBitcodeFromRlib> {
// We're about to assume the data here is an object file with sections, but if it's raw LLVM IR that
// won't work. Fortunately, if that's what we have we can just return the object directly, so we sniff
// the relevant magic strings here and return.
// We're about to assume the data here is an object file with sections, but if it's raw LLVM IR
// that won't work. Fortunately, if that's what we have we can just return the object directly,
// so we sniff the relevant magic strings here and return.
if obj.starts_with(b"\xDE\xC0\x17\x0B") || obj.starts_with(b"BC\xC0\xDE") {
return Ok(obj);
}
// We drop the "__LLVM," prefix here because on Apple platforms there's a notion of "segment name"
// which in the public API for sections gets treated as part of the section name, but internally
// in MachOObjectFile.cpp gets treated separately.
// We drop the "__LLVM," prefix here because on Apple platforms there's a notion of "segment
// name" which in the public API for sections gets treated as part of the section name, but
// internally in MachOObjectFile.cpp gets treated separately.
let section_name = bitcode_section_name(cgcx).trim_start_matches("__LLVM,");
let mut len = 0;
let data = unsafe {
Expand Down
12 changes: 7 additions & 5 deletions compiler/rustc_codegen_llvm/src/back/owned_target_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl OwnedTargetMachine {
data_sections: bool,
unique_section_names: bool,
trap_unreachable: bool,
singletree: bool,
singlethread: bool,
verbose_asm: bool,
emit_stack_size_section: bool,
relax_elf_relocations: bool,
Expand Down Expand Up @@ -62,7 +62,7 @@ impl OwnedTargetMachine {
data_sections,
unique_section_names,
trap_unreachable,
singletree,
singlethread,
verbose_asm,
emit_stack_size_section,
relax_elf_relocations,
Expand All @@ -86,15 +86,17 @@ impl Deref for OwnedTargetMachine {
type Target = llvm::TargetMachine;

fn deref(&self) -> &Self::Target {
// SAFETY: constructing ensures we have a valid pointer created by llvm::LLVMRustCreateTargetMachine
// SAFETY: constructing ensures we have a valid pointer created by
// llvm::LLVMRustCreateTargetMachine.
unsafe { self.tm_unique.as_ref() }
}
}

impl Drop for OwnedTargetMachine {
fn drop(&mut self) {
// SAFETY: constructing ensures we have a valid pointer created by llvm::LLVMRustCreateTargetMachine
// OwnedTargetMachine is not copyable so there is no double free or use after free
// SAFETY: constructing ensures we have a valid pointer created by
// llvm::LLVMRustCreateTargetMachine OwnedTargetMachine is not copyable so there is no
// double free or use after free.
unsafe {
llvm::LLVMRustDisposeTargetMachine(self.tm_unique.as_mut());
}
Expand Down
33 changes: 17 additions & 16 deletions compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::errors::{
CopyBitcode, FromLlvmDiag, FromLlvmOptimizationDiag, LlvmError, UnknownCompression,
WithLlvmError, WriteBytecode,
};
use crate::llvm::diagnostic::OptimizationDiagnosticKind;
use crate::llvm::diagnostic::OptimizationDiagnosticKind::*;
use crate::llvm::{self, DiagnosticInfo, PassManager};
use crate::type_::Type;
use crate::{base, common, llvm_util, LlvmCodegenBackend, ModuleLlvm};
Expand Down Expand Up @@ -157,7 +157,8 @@ fn to_pass_builder_opt_level(cfg: config::OptLevel) -> llvm::PassBuilderOptLevel
fn to_llvm_relocation_model(relocation_model: RelocModel) -> llvm::RelocModel {
match relocation_model {
RelocModel::Static => llvm::RelocModel::Static,
// LLVM doesn't have a PIE relocation model, it represents PIE as PIC with an extra attribute.
// LLVM doesn't have a PIE relocation model, it represents PIE as PIC with an extra
// attribute.
RelocModel::Pic | RelocModel::Pie => llvm::RelocModel::PIC,
RelocModel::DynamicNoPic => llvm::RelocModel::DynamicNoPic,
RelocModel::Ropi => llvm::RelocModel::ROPI,
Expand Down Expand Up @@ -188,8 +189,8 @@ pub(crate) fn target_machine_factory(
let use_softfp = if sess.target.arch == "arm" && sess.target.abi == "eabihf" {
sess.opts.cg.soft_float
} else {
// `validate_commandline_args_with_session_available` has already warned about this being ignored.
// Let's make sure LLVM doesn't suddenly start using this flag on more targets.
// `validate_commandline_args_with_session_available` has already warned about this being
// ignored. Let's make sure LLVM doesn't suddenly start using this flag on more targets.
false
};

Expand Down Expand Up @@ -446,13 +447,12 @@ unsafe extern "C" fn diagnostic_handler(info: &DiagnosticInfo, user: *mut c_void
column: opt.column,
pass_name: &opt.pass_name,
kind: match opt.kind {
OptimizationDiagnosticKind::OptimizationRemark => "success",
OptimizationDiagnosticKind::OptimizationMissed
| OptimizationDiagnosticKind::OptimizationFailure => "missed",
OptimizationDiagnosticKind::OptimizationAnalysis
| OptimizationDiagnosticKind::OptimizationAnalysisFPCommute
| OptimizationDiagnosticKind::OptimizationAnalysisAliasing => "analysis",
OptimizationDiagnosticKind::OptimizationRemarkOther => "other",
OptimizationRemark => "success",
OptimizationMissed | OptimizationFailure => "missed",
OptimizationAnalysis
| OptimizationAnalysisFPCommute
| OptimizationAnalysisAliasing => "analysis",
OptimizationRemarkOther => "other",
},
message: &opt.message,
});
Expand Down Expand Up @@ -945,11 +945,12 @@ fn create_section_with_flags_asm(section_name: &str, section_flags: &str, data:
}

fn target_is_apple(cgcx: &CodegenContext<LlvmCodegenBackend>) -> bool {
cgcx.opts.target_triple.triple().contains("-ios")
|| cgcx.opts.target_triple.triple().contains("-darwin")
|| cgcx.opts.target_triple.triple().contains("-tvos")
|| cgcx.opts.target_triple.triple().contains("-watchos")
|| cgcx.opts.target_triple.triple().contains("-visionos")
let triple = cgcx.opts.target_triple.triple();
triple.contains("-ios")
|| triple.contains("-darwin")
|| triple.contains("-tvos")
|| triple.contains("-watchos")
|| triple.contains("-visionos")
}

fn target_is_aix(cgcx: &CodegenContext<LlvmCodegenBackend>) -> bool {
Expand Down
117 changes: 32 additions & 85 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<'ll, 'tcx> Deref for Builder<'_, 'll, 'tcx> {
}
}

macro_rules! builder_methods_for_value_instructions {
macro_rules! math_builder_methods {
($($name:ident($($arg:ident),*) => $llvm_capi:ident),+ $(,)?) => {
$(fn $name(&mut self, $($arg: &'ll Value),*) -> &'ll Value {
unsafe {
Expand All @@ -130,6 +130,18 @@ macro_rules! builder_methods_for_value_instructions {
}
}

macro_rules! set_math_builder_methods {
($($name:ident($($arg:ident),*) => ($llvm_capi:ident, $llvm_set_math:ident)),+ $(,)?) => {
$(fn $name(&mut self, $($arg: &'ll Value),*) -> &'ll Value {
unsafe {
let instr = llvm::$llvm_capi(self.llbuilder, $($arg,)* UNNAMED);
llvm::$llvm_set_math(instr);
instr
}
})+
}
}

impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
type CodegenCx = CodegenCx<'ll, 'tcx>;

Expand Down Expand Up @@ -267,7 +279,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
}
}

builder_methods_for_value_instructions! {
math_builder_methods! {
add(a, b) => LLVMBuildAdd,
fadd(a, b) => LLVMBuildFAdd,
sub(a, b) => LLVMBuildSub,
Expand Down Expand Up @@ -299,84 +311,17 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
unchecked_umul(x, y) => LLVMBuildNUWMul,
}

fn fadd_fast(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFAdd(self.llbuilder, lhs, rhs, UNNAMED);
llvm::LLVMRustSetFastMath(instr);
instr
}
}

fn fsub_fast(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFSub(self.llbuilder, lhs, rhs, UNNAMED);
llvm::LLVMRustSetFastMath(instr);
instr
}
}

fn fmul_fast(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFMul(self.llbuilder, lhs, rhs, UNNAMED);
llvm::LLVMRustSetFastMath(instr);
instr
}
}

fn fdiv_fast(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFDiv(self.llbuilder, lhs, rhs, UNNAMED);
llvm::LLVMRustSetFastMath(instr);
instr
}
}

fn frem_fast(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFRem(self.llbuilder, lhs, rhs, UNNAMED);
llvm::LLVMRustSetFastMath(instr);
instr
}
}

fn fadd_algebraic(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFAdd(self.llbuilder, lhs, rhs, UNNAMED);
llvm::LLVMRustSetAlgebraicMath(instr);
instr
}
}

fn fsub_algebraic(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFSub(self.llbuilder, lhs, rhs, UNNAMED);
llvm::LLVMRustSetAlgebraicMath(instr);
instr
}
}

fn fmul_algebraic(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFMul(self.llbuilder, lhs, rhs, UNNAMED);
llvm::LLVMRustSetAlgebraicMath(instr);
instr
}
}

fn fdiv_algebraic(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFDiv(self.llbuilder, lhs, rhs, UNNAMED);
llvm::LLVMRustSetAlgebraicMath(instr);
instr
}
}

fn frem_algebraic(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
unsafe {
let instr = llvm::LLVMBuildFRem(self.llbuilder, lhs, rhs, UNNAMED);
llvm::LLVMRustSetAlgebraicMath(instr);
instr
}
set_math_builder_methods! {
fadd_fast(x, y) => (LLVMBuildFAdd, LLVMRustSetFastMath),
fsub_fast(x, y) => (LLVMBuildFSub, LLVMRustSetFastMath),
fmul_fast(x, y) => (LLVMBuildFMul, LLVMRustSetFastMath),
fdiv_fast(x, y) => (LLVMBuildFDiv, LLVMRustSetFastMath),
frem_fast(x, y) => (LLVMBuildFRem, LLVMRustSetFastMath),
fadd_algebraic(x, y) => (LLVMBuildFAdd, LLVMRustSetAlgebraicMath),
fsub_algebraic(x, y) => (LLVMBuildFSub, LLVMRustSetAlgebraicMath),
fmul_algebraic(x, y) => (LLVMBuildFMul, LLVMRustSetAlgebraicMath),
fdiv_algebraic(x, y) => (LLVMBuildFDiv, LLVMRustSetAlgebraicMath),
frem_algebraic(x, y) => (LLVMBuildFRem, LLVMRustSetAlgebraicMath),
}

fn checked_binop(
Expand Down Expand Up @@ -459,6 +404,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
val
}
}

fn to_immediate_scalar(&mut self, val: Self::Value, scalar: abi::Scalar) -> Self::Value {
if scalar.is_bool() {
return self.trunc(val, self.cx().type_i1());
Expand Down Expand Up @@ -727,11 +673,11 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
// for performance. LLVM doesn't seem to care about this, and will happily treat
// `!nontemporal` stores as-if they were normal stores (for reordering optimizations
// etc) even on x86, despite later lowering them to MOVNT which do *not* behave like
// regular stores but require special fences.
// So we keep a list of architectures where `!nontemporal` is known to be truly just
// a hint, and use regular stores everywhere else.
// (In the future, we could alternatively ensure that an sfence gets emitted after a sequence of movnt
// before any kind of synchronizing operation. But it's not clear how to do that with LLVM.)
// regular stores but require special fences. So we keep a list of architectures
// where `!nontemporal` is known to be truly just a hint, and use regular stores
// everywhere else. (In the future, we could alternatively ensure that an sfence
// gets emitted after a sequence of movnt before any kind of synchronizing
// operation. But it's not clear how to do that with LLVM.)
// For more context, see <https://github.com/rust-lang/rust/issues/114582> and
// <https://github.com/llvm/llvm-project/issues/64521>.
const WELL_BEHAVED_NONTEMPORAL_ARCHS: &[&str] =
Expand Down Expand Up @@ -1160,6 +1106,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
(val, success)
}
}

fn atomic_rmw(
&mut self,
op: rustc_codegen_ssa::common::AtomicRmwBinOp,
Expand Down
Loading

0 comments on commit 1a5a224

Please sign in to comment.