From a13be655a511c215cc4d1d120d981bfe80b04eec Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 28 Jun 2023 08:14:29 +1000 Subject: Avoid unnecessary line lookup. `lookup_debug_loc` calls `SourceMap::lookup_line`, which does a binary search over the files, and then a binary search over the lines within the found file. It then calls `SourceFile::line_begin_pos`, which redoes the binary search over the lines within the found file. This commit removes the second binary search over the lines, instead getting the line starting pos directly using the result of the first binary search over the lines. (And likewise for `get_span_loc`, in the cranelift backend.) --- compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'compiler/rustc_codegen_llvm') diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index aa7ae9355bc..44e591ceef1 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -262,7 +262,7 @@ impl CodegenCx<'_, '_> { pub fn lookup_debug_loc(&self, pos: BytePos) -> DebugLoc { let (file, line, col) = match self.sess().source_map().lookup_line(pos) { Ok(SourceFileAndLine { sf: file, line }) => { - let line_pos = file.line_begin_pos(pos); + let line_pos = file.lines(|lines| lines[line]); // Use 1-based indexing. let line = (line + 1) as u32; -- cgit 1.4.1-3-g733a5 From b4c6e19adeffad05e6039c21fdbda2097f3a2485 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 28 Jun 2023 08:57:32 +1000 Subject: Replace a `lookup_debug_loc` call. `lookup_debug_loc` finds a file, line, and column, which requires two binary searches. But this call site only needs the file. This commit replaces the call with `lookup_source_file`, which does a single binary search. --- compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'compiler/rustc_codegen_llvm') diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs b/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs index 64961baf272..65cbd5edc59 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs @@ -65,10 +65,10 @@ fn make_mir_scope<'ll, 'tcx>( debug_context.scopes[parent] } else { // The root is the function itself. - let loc = cx.lookup_debug_loc(mir.span.lo()); + let file = cx.sess().source_map().lookup_source_file(mir.span.lo()); debug_context.scopes[scope] = DebugScope { - file_start_pos: loc.file.start_pos, - file_end_pos: loc.file.end_pos, + file_start_pos: file.start_pos, + file_end_pos: file.end_pos, ..debug_context.scopes[scope] }; instantiated.insert(scope); -- cgit 1.4.1-3-g733a5 From de1914af342bbd1f20388f52d6ea342420de6fc0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 28 Jun 2023 11:30:27 +1000 Subject: Avoid an unnecessary use of `SmallStr`. I don't know why `SmallStr` was used here; some ad hoc profiling showed this code is not that hot, the string is usually empty, and when it's not empty it's usually very short. However, the use of a `SmallStr<1024>` does result in 1024 byte `memcpy` call on each execution, which shows up when I do `memcpy` profiling. So using a normal string makes the code both simpler and very slightly faster. --- compiler/rustc_codegen_llvm/src/attributes.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'compiler/rustc_codegen_llvm') diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs index 6d00464e0a0..39275272e42 100644 --- a/compiler/rustc_codegen_llvm/src/attributes.rs +++ b/compiler/rustc_codegen_llvm/src/attributes.rs @@ -1,7 +1,6 @@ //! Set and unset common attributes on LLVM values. use rustc_codegen_ssa::traits::*; -use rustc_data_structures::small_str::SmallStr; use rustc_hir::def_id::DefId; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::ty::{self, TyCtxt}; @@ -481,8 +480,8 @@ pub fn from_fn_attrs<'ll, 'tcx>( let global_features = cx.tcx.global_backend_features(()).iter().map(|s| s.as_str()); let function_features = function_features.iter().map(|s| s.as_str()); - let target_features = - global_features.chain(function_features).intersperse(",").collect::>(); + let target_features: String = + global_features.chain(function_features).intersperse(",").collect(); if !target_features.is_empty() { to_add.push(llvm::CreateAttrStringValue(cx.llcx, "target-features", &target_features)); } -- cgit 1.4.1-3-g733a5 From d20b1a8f6ba8ea753726fd70ce0e525f3bccffb9 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 28 Jun 2023 13:56:53 +1000 Subject: Set capacity of the string passed to `push_item_name`. Other callsites already do this, but these two were missed. This avoids some allocations. --- compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 2 +- compiler/rustc_codegen_llvm/src/debuginfo/namespace.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'compiler/rustc_codegen_llvm') diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 44e591ceef1..c2f16cad3fc 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -331,7 +331,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { llvm::LLVMRustDIBuilderCreateSubroutineType(DIB(self), fn_signature) }; - let mut name = String::new(); + let mut name = String::with_capacity(64); type_names::push_item_name(tcx, def_id, false, &mut name); // Find the enclosing function, in case this is a closure. diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/namespace.rs b/compiler/rustc_codegen_llvm/src/debuginfo/namespace.rs index d5ea48c311b..fa61c7dde18 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/namespace.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/namespace.rs @@ -28,7 +28,7 @@ pub fn item_namespace<'ll>(cx: &CodegenCx<'ll, '_>, def_id: DefId) -> &'ll DISco .map(|parent| item_namespace(cx, DefId { krate: def_id.krate, index: parent })); let namespace_name_string = { - let mut output = String::new(); + let mut output = String::with_capacity(64); type_names::push_item_name(cx.tcx, def_id, false, &mut output); output }; -- cgit 1.4.1-3-g733a5 From 81436ebd55e3512282bb3954e8fb2a936f1ef495 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 28 Jun 2023 14:07:43 +1000 Subject: Use `SmallVec` for the `bundles` vectors. They never have a length of more than two. So this commit changes them to `SmallVec<[_; 2]>`. Also, we possibly push `None` values and then filter those `None` values out again with `retain`. So this commit removes the `retain` and instead only pushes the values if they are `Some(_)`. --- compiler/rustc_codegen_llvm/src/builder.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) (limited to 'compiler/rustc_codegen_llvm') diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index b4aa001547c..2a4bb1709df 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -23,6 +23,7 @@ use rustc_span::Span; use rustc_symbol_mangling::typeid::{kcfi_typeid_for_fnabi, typeid_for_fnabi, TypeIdOptions}; use rustc_target::abi::{self, call::FnAbi, Align, Size, WrappingRange}; use rustc_target::spec::{HasTargetSpec, SanitizerSet, Target}; +use smallvec::SmallVec; use std::borrow::Cow; use std::iter; use std::ops::Deref; @@ -225,7 +226,10 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { let args = self.check_call("invoke", llty, llfn, args); let funclet_bundle = funclet.map(|funclet| funclet.bundle()); let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw); - let mut bundles = vec![funclet_bundle]; + let mut bundles: SmallVec<[_; 2]> = SmallVec::new(); + if funclet_bundle.is_some() { + bundles.push(funclet_bundle); + } // Emit CFI pointer type membership test self.cfi_type_test(fn_attrs, fn_abi, llfn); @@ -233,9 +237,10 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { // Emit KCFI operand bundle let kcfi_bundle = self.kcfi_operand_bundle(fn_attrs, fn_abi, llfn); let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw); - bundles.push(kcfi_bundle); + if kcfi_bundle.is_some() { + bundles.push(kcfi_bundle); + } - bundles.retain(|bundle| bundle.is_some()); let invoke = unsafe { llvm::LLVMRustBuildInvoke( self.llbuilder, @@ -1189,7 +1194,10 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { let args = self.check_call("call", llty, llfn, args); let funclet_bundle = funclet.map(|funclet| funclet.bundle()); let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw); - let mut bundles = vec![funclet_bundle]; + let mut bundles: SmallVec<[_; 2]> = SmallVec::new(); + if funclet_bundle.is_some() { + bundles.push(funclet_bundle); + } // Emit CFI pointer type membership test self.cfi_type_test(fn_attrs, fn_abi, llfn); @@ -1197,9 +1205,10 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { // Emit KCFI operand bundle let kcfi_bundle = self.kcfi_operand_bundle(fn_attrs, fn_abi, llfn); let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw); - bundles.push(kcfi_bundle); + if kcfi_bundle.is_some() { + bundles.push(kcfi_bundle); + } - bundles.retain(|bundle| bundle.is_some()); let call = unsafe { llvm::LLVMRustBuildCall( self.llbuilder, -- cgit 1.4.1-3-g733a5 From 8d7084d65fc3c1956b0c1459d26cb5fa9b811525 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 28 Jun 2023 14:09:51 +1000 Subject: Simplify the `bundles` vectors. After the last commit, they contain `Option<&OperandBundleDef<'a>>` but the values are always `Some(_)`. This commit removes the needless `Option` wrapper. This also simplifies the type signatures of `LLVMRustBuild{Invoke,Call}`, which were relying on the fact that the represention of `Option<&T>` is the same as `&T` for non-`None` values. --- compiler/rustc_codegen_llvm/src/builder.rs | 8 ++++---- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'compiler/rustc_codegen_llvm') diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 2a4bb1709df..3df7a7c9c5a 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -227,7 +227,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { let funclet_bundle = funclet.map(|funclet| funclet.bundle()); let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw); let mut bundles: SmallVec<[_; 2]> = SmallVec::new(); - if funclet_bundle.is_some() { + if let Some(funclet_bundle) = funclet_bundle { bundles.push(funclet_bundle); } @@ -237,7 +237,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { // Emit KCFI operand bundle let kcfi_bundle = self.kcfi_operand_bundle(fn_attrs, fn_abi, llfn); let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw); - if kcfi_bundle.is_some() { + if let Some(kcfi_bundle) = kcfi_bundle { bundles.push(kcfi_bundle); } @@ -1195,7 +1195,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { let funclet_bundle = funclet.map(|funclet| funclet.bundle()); let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw); let mut bundles: SmallVec<[_; 2]> = SmallVec::new(); - if funclet_bundle.is_some() { + if let Some(funclet_bundle) = funclet_bundle { bundles.push(funclet_bundle); } @@ -1205,7 +1205,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { // Emit KCFI operand bundle let kcfi_bundle = self.kcfi_operand_bundle(fn_attrs, fn_abi, llfn); let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw); - if kcfi_bundle.is_some() { + if let Some(kcfi_bundle) = kcfi_bundle { bundles.push(kcfi_bundle); } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 6ef3418cc5f..fdc5f3b193e 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1301,7 +1301,7 @@ extern "C" { NumArgs: c_uint, Then: &'a BasicBlock, Catch: &'a BasicBlock, - OpBundles: *const Option<&OperandBundleDef<'a>>, + OpBundles: *const &OperandBundleDef<'a>, NumOpBundles: c_uint, Name: *const c_char, ) -> &'a Value; @@ -1673,7 +1673,7 @@ extern "C" { Fn: &'a Value, Args: *const &'a Value, NumArgs: c_uint, - OpBundles: *const Option<&OperandBundleDef<'a>>, + OpBundles: *const &OperandBundleDef<'a>, NumOpBundles: c_uint, ) -> &'a Value; pub fn LLVMRustBuildMemCpy<'a>( -- cgit 1.4.1-3-g733a5