From 129b5e48f029779c57b46c72add9db03678d7cb5 Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Fri, 15 Mar 2024 15:53:49 -0400 Subject: [PATCH 1/2] avoid naming LLVM basic blocks when fewer_names is true --- compiler/rustc_codegen_gcc/src/builder.rs | 9 +++++---- compiler/rustc_codegen_llvm/src/builder.rs | 19 +++++++++++++++---- compiler/rustc_codegen_llvm/src/lib.rs | 1 + compiler/rustc_codegen_ssa/src/mir/block.rs | 12 +++++++----- .../rustc_codegen_ssa/src/traits/builder.rs | 10 ++++++++-- 5 files changed, 36 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/builder.rs b/compiler/rustc_codegen_gcc/src/builder.rs index f5cda81f6ab86..c12fa1a58fff8 100644 --- a/compiler/rustc_codegen_gcc/src/builder.rs +++ b/compiler/rustc_codegen_gcc/src/builder.rs @@ -1,6 +1,7 @@ use std::borrow::Cow; use std::cell::Cell; use std::convert::TryFrom; +use std::fmt::Display; use std::ops::Deref; use gccjit::{ @@ -526,14 +527,14 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> { self.block } - fn append_block(cx: &'a CodegenCx<'gcc, 'tcx>, func: RValue<'gcc>, name: &str) -> Block<'gcc> { + fn append_block(cx: &'a CodegenCx<'gcc, 'tcx>, func: RValue<'gcc>, name: impl Display) -> Block<'gcc> { let func = cx.rvalue_as_function(func); - func.new_block(name) + func.new_block(name.to_string()) } - fn append_sibling_block(&mut self, name: &str) -> Block<'gcc> { + fn append_sibling_block(&mut self, name: impl Display) -> Block<'gcc> { let func = self.current_func(); - func.new_block(name) + func.new_block(name.to_string()) } fn switch_to_block(&mut self, block: Self::BasicBlock) { diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 63e59ea13fc35..b244ed959f9e8 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -26,6 +26,7 @@ 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::fmt::Display; use std::iter; use std::ops::Deref; use std::ptr; @@ -153,14 +154,24 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { fn set_span(&mut self, _span: Span) {} - fn append_block(cx: &'a CodegenCx<'ll, 'tcx>, llfn: &'ll Value, name: &str) -> &'ll BasicBlock { + fn append_block( + cx: &'a CodegenCx<'ll, 'tcx>, + llfn: &'ll Value, + name: impl Display, + ) -> &'ll BasicBlock { unsafe { - let name = SmallCStr::new(name); - llvm::LLVMAppendBasicBlockInContext(cx.llcx, llfn, name.as_ptr()) + let c_str_name; + let name_ptr = if cx.tcx.sess.fewer_names() { + const { c"".as_ptr().cast() } + } else { + c_str_name = SmallCStr::new(&name.to_string()); + c_str_name.as_ptr() + }; + llvm::LLVMAppendBasicBlockInContext(cx.llcx, llfn, name_ptr) } } - fn append_sibling_block(&mut self, name: &str) -> &'ll BasicBlock { + fn append_sibling_block(&mut self, name: impl Display) -> &'ll BasicBlock { Self::append_block(self.cx, self.llfn(), name) } diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index c84461e53eb1b..72e5e2b042ee9 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -11,6 +11,7 @@ #![feature(exact_size_is_empty)] #![feature(extern_types)] #![feature(hash_raw_entry)] +#![feature(inline_const)] #![feature(iter_intersperse)] #![feature(let_chains)] #![feature(impl_trait_in_assoc_type)] diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 9bb2a52826585..3db6bc5b42643 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -83,8 +83,11 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { // Cross-funclet jump - need a trampoline debug_assert!(base::wants_new_eh_instructions(fx.cx.tcx().sess)); debug!("llbb_with_cleanup: creating cleanup trampoline for {:?}", target); - let name = &format!("{:?}_cleanup_trampoline_{:?}", self.bb, target); - let trampoline_llbb = Bx::append_block(fx.cx, fx.llfn, name); + let trampoline_llbb = Bx::append_block( + fx.cx, + fx.llfn, + format_args!("{:?}_cleanup_trampoline_{:?}", self.bb, target), + ); let mut trampoline_bx = Bx::build(fx.cx, trampoline_llbb); trampoline_bx.cleanup_ret(self.funclet(fx).unwrap(), Some(lltarget)); trampoline_llbb @@ -1565,7 +1568,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { fn landing_pad_for_uncached(&mut self, bb: mir::BasicBlock) -> Bx::BasicBlock { let llbb = self.llbb(bb); if base::wants_new_eh_instructions(self.cx.sess()) { - let cleanup_bb = Bx::append_block(self.cx, self.llfn, &format!("funclet_{bb:?}")); + let cleanup_bb = Bx::append_block(self.cx, self.llfn, format_args!("funclet_{bb:?}")); let mut cleanup_bx = Bx::build(self.cx, cleanup_bb); let funclet = cleanup_bx.cleanup_pad(None, &[]); cleanup_bx.br(llbb); @@ -1688,8 +1691,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { pub fn try_llbb(&mut self, bb: mir::BasicBlock) -> Option { match self.cached_llbbs[bb] { CachedLlbb::None => { - // FIXME(eddyb) only name the block if `fewer_names` is `false`. - let llbb = Bx::append_block(self.cx, self.llfn, &format!("{bb:?}")); + let llbb = Bx::append_block(self.cx, self.llfn, format_args!("{bb:?}")); self.cached_llbbs[bb] = CachedLlbb::Some(llbb); Some(llbb) } diff --git a/compiler/rustc_codegen_ssa/src/traits/builder.rs b/compiler/rustc_codegen_ssa/src/traits/builder.rs index 36f37e3791bc5..49c3101bc5e51 100644 --- a/compiler/rustc_codegen_ssa/src/traits/builder.rs +++ b/compiler/rustc_codegen_ssa/src/traits/builder.rs @@ -22,6 +22,8 @@ use rustc_target::abi::call::FnAbi; use rustc_target::abi::{Abi, Align, Scalar, Size, WrappingRange}; use rustc_target::spec::HasTargetSpec; +use std::fmt::Display; + #[derive(Copy, Clone)] pub enum OverflowOp { Add, @@ -49,9 +51,13 @@ pub trait BuilderMethods<'a, 'tcx>: fn set_span(&mut self, span: Span); // FIXME(eddyb) replace uses of this with `append_sibling_block`. - fn append_block(cx: &'a Self::CodegenCx, llfn: Self::Function, name: &str) -> Self::BasicBlock; + fn append_block( + cx: &'a Self::CodegenCx, + llfn: Self::Function, + name: impl Display, + ) -> Self::BasicBlock; - fn append_sibling_block(&mut self, name: &str) -> Self::BasicBlock; + fn append_sibling_block(&mut self, name: impl Display) -> Self::BasicBlock; fn switch_to_block(&mut self, llbb: Self::BasicBlock); From a7d4258e00543a9a62fcfabbed4f27121f8f48ce Mon Sep 17 00:00:00 2001 From: Erik Desjardins Date: Sat, 16 Mar 2024 11:11:53 -0400 Subject: [PATCH 2/2] revert changes and just delete the fixme Avoiding the naming didn't have any meaningful perf impact. --- compiler/rustc_codegen_gcc/src/builder.rs | 9 ++++----- compiler/rustc_codegen_llvm/src/builder.rs | 19 ++++--------------- compiler/rustc_codegen_llvm/src/lib.rs | 1 - compiler/rustc_codegen_ssa/src/mir/block.rs | 11 ++++------- .../rustc_codegen_ssa/src/traits/builder.rs | 10 ++-------- 5 files changed, 14 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/builder.rs b/compiler/rustc_codegen_gcc/src/builder.rs index c12fa1a58fff8..f5cda81f6ab86 100644 --- a/compiler/rustc_codegen_gcc/src/builder.rs +++ b/compiler/rustc_codegen_gcc/src/builder.rs @@ -1,7 +1,6 @@ use std::borrow::Cow; use std::cell::Cell; use std::convert::TryFrom; -use std::fmt::Display; use std::ops::Deref; use gccjit::{ @@ -527,14 +526,14 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> { self.block } - fn append_block(cx: &'a CodegenCx<'gcc, 'tcx>, func: RValue<'gcc>, name: impl Display) -> Block<'gcc> { + fn append_block(cx: &'a CodegenCx<'gcc, 'tcx>, func: RValue<'gcc>, name: &str) -> Block<'gcc> { let func = cx.rvalue_as_function(func); - func.new_block(name.to_string()) + func.new_block(name) } - fn append_sibling_block(&mut self, name: impl Display) -> Block<'gcc> { + fn append_sibling_block(&mut self, name: &str) -> Block<'gcc> { let func = self.current_func(); - func.new_block(name.to_string()) + func.new_block(name) } fn switch_to_block(&mut self, block: Self::BasicBlock) { diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index b244ed959f9e8..63e59ea13fc35 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -26,7 +26,6 @@ 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::fmt::Display; use std::iter; use std::ops::Deref; use std::ptr; @@ -154,24 +153,14 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { fn set_span(&mut self, _span: Span) {} - fn append_block( - cx: &'a CodegenCx<'ll, 'tcx>, - llfn: &'ll Value, - name: impl Display, - ) -> &'ll BasicBlock { + fn append_block(cx: &'a CodegenCx<'ll, 'tcx>, llfn: &'ll Value, name: &str) -> &'ll BasicBlock { unsafe { - let c_str_name; - let name_ptr = if cx.tcx.sess.fewer_names() { - const { c"".as_ptr().cast() } - } else { - c_str_name = SmallCStr::new(&name.to_string()); - c_str_name.as_ptr() - }; - llvm::LLVMAppendBasicBlockInContext(cx.llcx, llfn, name_ptr) + let name = SmallCStr::new(name); + llvm::LLVMAppendBasicBlockInContext(cx.llcx, llfn, name.as_ptr()) } } - fn append_sibling_block(&mut self, name: impl Display) -> &'ll BasicBlock { + fn append_sibling_block(&mut self, name: &str) -> &'ll BasicBlock { Self::append_block(self.cx, self.llfn(), name) } diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 72e5e2b042ee9..c84461e53eb1b 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -11,7 +11,6 @@ #![feature(exact_size_is_empty)] #![feature(extern_types)] #![feature(hash_raw_entry)] -#![feature(inline_const)] #![feature(iter_intersperse)] #![feature(let_chains)] #![feature(impl_trait_in_assoc_type)] diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 3db6bc5b42643..02e7bb05b7709 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -83,11 +83,8 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> { // Cross-funclet jump - need a trampoline debug_assert!(base::wants_new_eh_instructions(fx.cx.tcx().sess)); debug!("llbb_with_cleanup: creating cleanup trampoline for {:?}", target); - let trampoline_llbb = Bx::append_block( - fx.cx, - fx.llfn, - format_args!("{:?}_cleanup_trampoline_{:?}", self.bb, target), - ); + let name = &format!("{:?}_cleanup_trampoline_{:?}", self.bb, target); + let trampoline_llbb = Bx::append_block(fx.cx, fx.llfn, name); let mut trampoline_bx = Bx::build(fx.cx, trampoline_llbb); trampoline_bx.cleanup_ret(self.funclet(fx).unwrap(), Some(lltarget)); trampoline_llbb @@ -1568,7 +1565,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { fn landing_pad_for_uncached(&mut self, bb: mir::BasicBlock) -> Bx::BasicBlock { let llbb = self.llbb(bb); if base::wants_new_eh_instructions(self.cx.sess()) { - let cleanup_bb = Bx::append_block(self.cx, self.llfn, format_args!("funclet_{bb:?}")); + let cleanup_bb = Bx::append_block(self.cx, self.llfn, &format!("funclet_{bb:?}")); let mut cleanup_bx = Bx::build(self.cx, cleanup_bb); let funclet = cleanup_bx.cleanup_pad(None, &[]); cleanup_bx.br(llbb); @@ -1691,7 +1688,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { pub fn try_llbb(&mut self, bb: mir::BasicBlock) -> Option { match self.cached_llbbs[bb] { CachedLlbb::None => { - let llbb = Bx::append_block(self.cx, self.llfn, format_args!("{bb:?}")); + let llbb = Bx::append_block(self.cx, self.llfn, &format!("{bb:?}")); self.cached_llbbs[bb] = CachedLlbb::Some(llbb); Some(llbb) } diff --git a/compiler/rustc_codegen_ssa/src/traits/builder.rs b/compiler/rustc_codegen_ssa/src/traits/builder.rs index 49c3101bc5e51..36f37e3791bc5 100644 --- a/compiler/rustc_codegen_ssa/src/traits/builder.rs +++ b/compiler/rustc_codegen_ssa/src/traits/builder.rs @@ -22,8 +22,6 @@ use rustc_target::abi::call::FnAbi; use rustc_target::abi::{Abi, Align, Scalar, Size, WrappingRange}; use rustc_target::spec::HasTargetSpec; -use std::fmt::Display; - #[derive(Copy, Clone)] pub enum OverflowOp { Add, @@ -51,13 +49,9 @@ pub trait BuilderMethods<'a, 'tcx>: fn set_span(&mut self, span: Span); // FIXME(eddyb) replace uses of this with `append_sibling_block`. - fn append_block( - cx: &'a Self::CodegenCx, - llfn: Self::Function, - name: impl Display, - ) -> Self::BasicBlock; + fn append_block(cx: &'a Self::CodegenCx, llfn: Self::Function, name: &str) -> Self::BasicBlock; - fn append_sibling_block(&mut self, name: impl Display) -> Self::BasicBlock; + fn append_sibling_block(&mut self, name: &str) -> Self::BasicBlock; fn switch_to_block(&mut self, llbb: Self::BasicBlock);