From adfcae2e0a9fdc04bcd39a3406ff1acf6cea05a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 25 Jun 2023 23:39:02 +0200 Subject: [PATCH 01/10] Add `rustc` option to output LLVM optimization remarks to YAML files --- compiler/rustc_codegen_llvm/src/back/lto.rs | 10 ++- compiler/rustc_codegen_llvm/src/back/write.rs | 39 +++++++++- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 1 + compiler/rustc_codegen_ssa/src/back/write.rs | 14 ++++ .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 74 +++++++++++++++++-- compiler/rustc_session/src/config.rs | 4 + compiler/rustc_session/src/options.rs | 5 +- 7 files changed, 134 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs index 8b05af7bed909..a31551f307911 100644 --- a/compiler/rustc_codegen_llvm/src/back/lto.rs +++ b/compiler/rustc_codegen_llvm/src/back/lto.rs @@ -1,4 +1,4 @@ -use crate::back::write::{self, save_temp_bitcode, DiagnosticHandlers}; +use crate::back::write::{self, save_temp_bitcode, CodegenDiagnosticsSection, DiagnosticHandlers}; use crate::errors::{ DynamicLinkingWithLTO, LlvmError, LtoBitcodeFromRlib, LtoDisallowed, LtoDylib, }; @@ -302,7 +302,13 @@ fn fat_lto( // The linking steps below may produce errors and diagnostics within LLVM // which we'd like to handle and print, so set up our diagnostic handlers // (which get unregistered when they go out of scope below). - let _handler = DiagnosticHandlers::new(cgcx, diag_handler, llcx); + let _handler = DiagnosticHandlers::new( + cgcx, + diag_handler, + llcx, + &module, + CodegenDiagnosticsSection::LTO, + ); // For all other modules we codegened we'll need to link them into our own // bitcode. All modules were codegened in their own LLVM context, however, diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index 53b4296802ef7..7bd7cbc2c8125 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -268,6 +268,13 @@ pub(crate) fn save_temp_bitcode( } } +/// In what context is a dignostic handler being attached to a codegen unit? +pub enum CodegenDiagnosticsSection { + Codegen, + Opt, + LTO, +} + pub struct DiagnosticHandlers<'a> { data: *mut (&'a CodegenContext, &'a Handler), llcx: &'a llvm::Context, @@ -279,6 +286,8 @@ impl<'a> DiagnosticHandlers<'a> { cgcx: &'a CodegenContext, handler: &'a Handler, llcx: &'a llvm::Context, + module: &ModuleCodegen, + section: CodegenDiagnosticsSection, ) -> Self { let remark_passes_all: bool; let remark_passes: Vec; @@ -295,6 +304,21 @@ impl<'a> DiagnosticHandlers<'a> { }; let remark_passes: Vec<*const c_char> = remark_passes.iter().map(|name: &CString| name.as_ptr()).collect(); + let remark_file = cgcx + .remark_dir + .as_ref() + .and_then(|dir| dir.to_str()) + // Use the .opt.yaml file pattern, which is supported by LLVM's opt-viewer. + .map(|dir| { + let section = match section { + CodegenDiagnosticsSection::Codegen => "codegen", + CodegenDiagnosticsSection::Opt => "opt", + CodegenDiagnosticsSection::LTO => "lto", + }; + format!("{dir}/{}.{section}.opt.yaml", module.name) + }) + .map(|dir| CString::new(dir).unwrap()); + let data = Box::into_raw(Box::new((cgcx, handler))); unsafe { let old_handler = llvm::LLVMRustContextGetDiagnosticHandler(llcx); @@ -305,6 +329,9 @@ impl<'a> DiagnosticHandlers<'a> { remark_passes_all, remark_passes.as_ptr(), remark_passes.len(), + // The `as_ref()` is important here, otherwise the `CString` will be dropped + // too soon! + remark_file.as_ref().map(|dir| dir.as_ptr()).unwrap_or(std::ptr::null()), ); DiagnosticHandlers { data, llcx, old_handler } } @@ -523,7 +550,8 @@ pub(crate) unsafe fn optimize( let llmod = module.module_llvm.llmod(); let llcx = &*module.module_llvm.llcx; - let _handlers = DiagnosticHandlers::new(cgcx, diag_handler, llcx); + let _handlers = + DiagnosticHandlers::new(cgcx, diag_handler, llcx, module, CodegenDiagnosticsSection::Opt); let module_name = module.name.clone(); let module_name = Some(&module_name[..]); @@ -582,7 +610,13 @@ pub(crate) unsafe fn codegen( let tm = &*module.module_llvm.tm; let module_name = module.name.clone(); let module_name = Some(&module_name[..]); - let handlers = DiagnosticHandlers::new(cgcx, diag_handler, llcx); + let _handlers = DiagnosticHandlers::new( + cgcx, + diag_handler, + llcx, + &module, + CodegenDiagnosticsSection::Codegen, + ); if cgcx.msvc_imps_needed { create_msvc_imps(cgcx, llcx, llmod); @@ -775,7 +809,6 @@ pub(crate) unsafe fn codegen( } record_llvm_cgu_instructions_stats(&cgcx.prof, llmod); - drop(handlers); } // `.dwo` files are only emitted if: diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 6ef3418cc5f77..56a560d6866e6 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -2512,6 +2512,7 @@ extern "C" { remark_all_passes: bool, remark_passes: *const *const c_char, remark_passes_len: usize, + remark_file: *const c_char, ); #[allow(improper_ctypes)] diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 51ac441a7a425..6b0df03c1fb9f 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -345,6 +345,9 @@ pub struct CodegenContext { pub diag_emitter: SharedEmitter, /// LLVM optimizations for which we want to print remarks. pub remark: Passes, + /// Directory into which should the LLVM optimization remarks be written. + /// If `None`, they will be written to stderr. + pub remark_dir: Option, /// Worker thread number pub worker: usize, /// The incremental compilation session directory, or None if we are not @@ -1041,6 +1044,16 @@ fn start_executing_work( tcx.backend_optimization_level(()) }; let backend_features = tcx.global_backend_features(()); + + let remark_dir = if let Some(ref dir) = sess.opts.unstable_opts.remark_dir { + // Should this be here? + // Can this conflict with a parallel attempt to create this directory? + fs::create_dir_all(dir).expect("Cannot create remark directory"); + Some(dir.canonicalize().expect("Cannot canonicalize remark directory")) + } else { + None + }; + let cgcx = CodegenContext:: { crate_types: sess.crate_types().to_vec(), each_linked_rlib_for_lto, @@ -1052,6 +1065,7 @@ fn start_executing_work( prof: sess.prof.clone(), exported_symbols, remark: sess.opts.cg.remark.clone(), + remark_dir, worker: 0, incr_comp_session_dir: sess.incr_comp_session_dir_opt().map(|r| r.clone()), cgu_reuse_tracker: sess.cgu_reuse_tracker.clone(), diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index ea04899ab6872..e00613d9f50ef 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -7,7 +7,12 @@ #include "llvm/IR/Instructions.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/IntrinsicsARM.h" +#include "llvm/IR/LLVMRemarkStreamer.h" #include "llvm/IR/Mangler.h" +#include "llvm/Remarks/RemarkStreamer.h" +#include "llvm/Remarks/RemarkSerializer.h" +#include "llvm/Remarks/RemarkFormat.h" +#include "llvm/Support/ToolOutputFile.h" #if LLVM_VERSION_GE(16, 0) #include "llvm/Support/ModRef.h" #endif @@ -1855,23 +1860,41 @@ using LLVMDiagnosticHandlerTy = DiagnosticHandler::DiagnosticHandlerTy; // When RemarkAllPasses is true, remarks are enabled for all passes. Otherwise // the RemarkPasses array specifies individual passes for which remarks will be // enabled. +// +// If RemarkFilePath is not NULL, optimization remarks will be streamed directly into this file, +// bypassing the diagnostics handler. extern "C" void LLVMRustContextConfigureDiagnosticHandler( LLVMContextRef C, LLVMDiagnosticHandlerTy DiagnosticHandlerCallback, void *DiagnosticHandlerContext, bool RemarkAllPasses, - const char * const * RemarkPasses, size_t RemarkPassesLen) { + const char * const * RemarkPasses, size_t RemarkPassesLen, + const char * RemarkFilePath +) { class RustDiagnosticHandler final : public DiagnosticHandler { public: - RustDiagnosticHandler(LLVMDiagnosticHandlerTy DiagnosticHandlerCallback, - void *DiagnosticHandlerContext, - bool RemarkAllPasses, - std::vector RemarkPasses) + RustDiagnosticHandler( + LLVMDiagnosticHandlerTy DiagnosticHandlerCallback, + void *DiagnosticHandlerContext, + bool RemarkAllPasses, + std::vector RemarkPasses, + std::unique_ptr RemarkStreamer, + std::unique_ptr RemarksFile + ) : DiagnosticHandlerCallback(DiagnosticHandlerCallback), DiagnosticHandlerContext(DiagnosticHandlerContext), RemarkAllPasses(RemarkAllPasses), - RemarkPasses(RemarkPasses) {} + RemarkPasses(std::move(RemarkPasses)), + RemarkStreamer(std::move(RemarkStreamer)), + RemarksFile(std::move(RemarksFile)) {} virtual bool handleDiagnostics(const DiagnosticInfo &DI) override { + if (this->RemarkStreamer) { + if (auto *OptDiagBase = dyn_cast(&DI)) { + LLVMRemarkStreamer Streamer(*this->RemarkStreamer); + Streamer.emit(*OptDiagBase); + return true; + } + } if (DiagnosticHandlerCallback) { DiagnosticHandlerCallback(DI, DiagnosticHandlerContext); return true; @@ -1912,14 +1935,51 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( bool RemarkAllPasses = false; std::vector RemarkPasses; + std::unique_ptr RemarkStreamer; + std::unique_ptr RemarksFile; }; std::vector Passes; for (size_t I = 0; I != RemarkPassesLen; ++I) + { Passes.push_back(RemarkPasses[I]); + } + + // We need to hold onto both the streamer and the opened file + std::unique_ptr RemarkStreamer; + std::unique_ptr RemarkFile; + + if (RemarkFilePath != nullptr) { + std::error_code EC; + RemarkFile = std::make_unique( + RemarkFilePath, + EC, + llvm::sys::fs::OF_TextWithCRLF + ); + // Do not delete the file after we gather remarks + RemarkFile->keep(); + + auto RemarkSerializer = remarks::createRemarkSerializer( + llvm::remarks::Format::YAML, + remarks::SerializerMode::Separate, + RemarkFile->os() + ); + if (Error E = RemarkSerializer.takeError()) + { + // Is this OK? + report_fatal_error("Cannot create remark serializer"); + } + RemarkStreamer = std::make_unique(std::move(*RemarkSerializer)); + } unwrap(C)->setDiagnosticHandler(std::make_unique( - DiagnosticHandlerCallback, DiagnosticHandlerContext, RemarkAllPasses, Passes)); + DiagnosticHandlerCallback, + DiagnosticHandlerContext, + RemarkAllPasses, + Passes, + std::move(RemarkStreamer), + std::move(RemarkFile) + )); } extern "C" void LLVMRustGetMangledName(LLVMValueRef V, RustStringRef Str) { diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 480d2478e8174..736dd4714b0e5 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2583,6 +2583,10 @@ pub fn build_session_options( handler.early_warn("-C remark requires \"-C debuginfo=n\" to show source locations"); } + if cg.remark.is_empty() && unstable_opts.remark_dir.is_some() { + handler.early_warn("using -C remark-dir without enabling remarks using e.g. -C remark=all"); + } + let externs = parse_externs(handler, matches, &unstable_opts); let crate_name = matches.opt_str("crate-name"); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index e5063eef47af5..210bcfec77602 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1314,7 +1314,7 @@ options! { "control generation of position-independent code (PIC) \ (`rustc --print relocation-models` for details)"), remark: Passes = (Passes::Some(Vec::new()), parse_passes, [UNTRACKED], - "print remarks for these optimization passes (space separated, or \"all\")"), + "output remarks for these optimization passes (space separated, or \"all\")"), rpath: bool = (false, parse_bool, [UNTRACKED], "set rpath values in libs/exes (default: no)"), save_temps: bool = (false, parse_bool, [UNTRACKED], @@ -1659,6 +1659,9 @@ options! { "choose which RELRO level to use"), remap_cwd_prefix: Option = (None, parse_opt_pathbuf, [TRACKED], "remap paths under the current working directory to this path prefix"), + remark_dir: Option = (None, parse_opt_pathbuf, [UNTRACKED], + "directory into which to write optimization remarks (if not specified, they will be \ +written to standard error output)"), report_delayed_bugs: bool = (false, parse_bool, [TRACKED], "immediately print bugs registered with `delay_span_bug` (default: no)"), sanitizer: SanitizerSet = (SanitizerSet::empty(), parse_sanitizers, [TRACKED], From 9a67df290c7d2a5b94d8095a13c472171202f544 Mon Sep 17 00:00:00 2001 From: Li Zhanhui Date: Thu, 29 Jun 2023 04:21:00 +0000 Subject: [PATCH 02/10] Fix document examples of Vec::from_raw_parts and Vec::from_raw_parts_in Signed-off-by: Li Zhanhui --- library/alloc/src/vec/mod.rs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index ef4f8be6f19de..598ecf05e8246 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -560,22 +560,20 @@ impl Vec { /// Using memory that was allocated elsewhere: /// /// ```rust - /// #![feature(allocator_api)] - /// - /// use std::alloc::{AllocError, Allocator, Global, Layout}; + /// use std::alloc::{alloc, Layout}; /// /// fn main() { /// let layout = Layout::array::(16).expect("overflow cannot happen"); /// /// let vec = unsafe { - /// let mem = match Global.allocate(layout) { - /// Ok(mem) => mem.cast::().as_ptr(), - /// Err(AllocError) => return, - /// }; + /// let mem = alloc(layout).cast::(); + /// if mem.is_null() { + /// return; + /// } /// /// mem.write(1_000_000); /// - /// Vec::from_raw_parts_in(mem, 1, 16, Global) + /// Vec::from_raw_parts(mem, 1, 16) /// }; /// /// assert_eq!(vec, &[1_000_000]); @@ -758,19 +756,22 @@ impl Vec { /// Using memory that was allocated elsewhere: /// /// ```rust - /// use std::alloc::{alloc, Layout}; + /// #![feature(allocator_api)] + /// + /// use std::alloc::{AllocError, Allocator, Global, Layout}; /// /// fn main() { /// let layout = Layout::array::(16).expect("overflow cannot happen"); + /// /// let vec = unsafe { - /// let mem = alloc(layout).cast::(); - /// if mem.is_null() { - /// return; - /// } + /// let mem = match Global.allocate(layout) { + /// Ok(mem) => mem.cast::().as_ptr(), + /// Err(AllocError) => return, + /// }; /// /// mem.write(1_000_000); /// - /// Vec::from_raw_parts(mem, 1, 16) + /// Vec::from_raw_parts_in(mem, 1, 16, Global) /// }; /// /// assert_eq!(vec, &[1_000_000]); From 6782bb42a66ebc4ca4c2a4117001b9137c3459e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 29 Jun 2023 11:41:24 +0200 Subject: [PATCH 03/10] Address review comments --- compiler/rustc_codegen_llvm/src/back/lto.rs | 4 +-- compiler/rustc_codegen_llvm/src/back/write.rs | 26 ++++++++++--------- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 20 ++++++++++---- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs index a31551f307911..94885b40cc1a1 100644 --- a/compiler/rustc_codegen_llvm/src/back/lto.rs +++ b/compiler/rustc_codegen_llvm/src/back/lto.rs @@ -1,4 +1,4 @@ -use crate::back::write::{self, save_temp_bitcode, CodegenDiagnosticsSection, DiagnosticHandlers}; +use crate::back::write::{self, save_temp_bitcode, CodegenDiagnosticsStage, DiagnosticHandlers}; use crate::errors::{ DynamicLinkingWithLTO, LlvmError, LtoBitcodeFromRlib, LtoDisallowed, LtoDylib, }; @@ -307,7 +307,7 @@ fn fat_lto( diag_handler, llcx, &module, - CodegenDiagnosticsSection::LTO, + CodegenDiagnosticsStage::LTO, ); // For all other modules we codegened we'll need to link them into our own diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index 7bd7cbc2c8125..a938bef8397f5 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -269,10 +269,13 @@ pub(crate) fn save_temp_bitcode( } /// In what context is a dignostic handler being attached to a codegen unit? -pub enum CodegenDiagnosticsSection { - Codegen, +pub enum CodegenDiagnosticsStage { + /// Prelink optimization stage. Opt, + /// LTO/ThinLTO postlink optimization stage. LTO, + /// Code generation. + Codegen, } pub struct DiagnosticHandlers<'a> { @@ -287,7 +290,7 @@ impl<'a> DiagnosticHandlers<'a> { handler: &'a Handler, llcx: &'a llvm::Context, module: &ModuleCodegen, - section: CodegenDiagnosticsSection, + section: CodegenDiagnosticsStage, ) -> Self { let remark_passes_all: bool; let remark_passes: Vec; @@ -307,17 +310,16 @@ impl<'a> DiagnosticHandlers<'a> { let remark_file = cgcx .remark_dir .as_ref() - .and_then(|dir| dir.to_str()) - // Use the .opt.yaml file pattern, which is supported by LLVM's opt-viewer. + // Use the .opt.yaml file suffix, which is supported by LLVM's opt-viewer. .map(|dir| { let section = match section { - CodegenDiagnosticsSection::Codegen => "codegen", - CodegenDiagnosticsSection::Opt => "opt", - CodegenDiagnosticsSection::LTO => "lto", + CodegenDiagnosticsStage::Codegen => "codegen", + CodegenDiagnosticsStage::Opt => "opt", + CodegenDiagnosticsStage::LTO => "lto", }; - format!("{dir}/{}.{section}.opt.yaml", module.name) + dir.join(format!("{}.{section}.opt.yaml", module.name)) }) - .map(|dir| CString::new(dir).unwrap()); + .and_then(|dir| dir.to_str().and_then(|p| CString::new(p).ok())); let data = Box::into_raw(Box::new((cgcx, handler))); unsafe { @@ -551,7 +553,7 @@ pub(crate) unsafe fn optimize( let llmod = module.module_llvm.llmod(); let llcx = &*module.module_llvm.llcx; let _handlers = - DiagnosticHandlers::new(cgcx, diag_handler, llcx, module, CodegenDiagnosticsSection::Opt); + DiagnosticHandlers::new(cgcx, diag_handler, llcx, module, CodegenDiagnosticsStage::Opt); let module_name = module.name.clone(); let module_name = Some(&module_name[..]); @@ -615,7 +617,7 @@ pub(crate) unsafe fn codegen( diag_handler, llcx, &module, - CodegenDiagnosticsSection::Codegen, + CodegenDiagnosticsStage::Codegen, ); if cgcx.msvc_imps_needed { diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index e00613d9f50ef..a3f149162545f 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1878,6 +1878,7 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( bool RemarkAllPasses, std::vector RemarkPasses, std::unique_ptr RemarkStreamer, + std::unique_ptr LlvmRemarkStreamer, std::unique_ptr RemarksFile ) : DiagnosticHandlerCallback(DiagnosticHandlerCallback), @@ -1885,14 +1886,16 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( RemarkAllPasses(RemarkAllPasses), RemarkPasses(std::move(RemarkPasses)), RemarkStreamer(std::move(RemarkStreamer)), + LlvmRemarkStreamer(std::move(LlvmRemarkStreamer)), RemarksFile(std::move(RemarksFile)) {} virtual bool handleDiagnostics(const DiagnosticInfo &DI) override { - if (this->RemarkStreamer) { + if (this->LlvmRemarkStreamer) { if (auto *OptDiagBase = dyn_cast(&DI)) { - LLVMRemarkStreamer Streamer(*this->RemarkStreamer); - Streamer.emit(*OptDiagBase); - return true; + if (OptDiagBase->isEnabled()) { + this->LlvmRemarkStreamer->emit(*OptDiagBase); + return true; + } } } if (DiagnosticHandlerCallback) { @@ -1935,7 +1938,11 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( bool RemarkAllPasses = false; std::vector RemarkPasses; + + // Since LlvmRemarkStreamer contains a pointer to RemarkStreamer, the ordering of the two + // members below is important. std::unique_ptr RemarkStreamer; + std::unique_ptr LlvmRemarkStreamer; std::unique_ptr RemarksFile; }; @@ -1945,8 +1952,9 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( Passes.push_back(RemarkPasses[I]); } - // We need to hold onto both the streamer and the opened file + // We need to hold onto both the streamers and the opened file std::unique_ptr RemarkStreamer; + std::unique_ptr LlvmRemarkStreamer; std::unique_ptr RemarkFile; if (RemarkFilePath != nullptr) { @@ -1970,6 +1978,7 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( report_fatal_error("Cannot create remark serializer"); } RemarkStreamer = std::make_unique(std::move(*RemarkSerializer)); + LlvmRemarkStreamer = std::make_unique(*RemarkStreamer); } unwrap(C)->setDiagnosticHandler(std::make_unique( @@ -1978,6 +1987,7 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( RemarkAllPasses, Passes, std::move(RemarkStreamer), + std::move(LlvmRemarkStreamer), std::move(RemarkFile) )); } From ce5ec54e94d4144e5209dc58d02366aae5eb8ddf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 29 Jun 2023 12:03:38 +0200 Subject: [PATCH 04/10] Add simple test --- tests/run-make/optimization-remarks-dir/Makefile | 12 ++++++++++++ tests/run-make/optimization-remarks-dir/main.rs | 6 ++++++ 2 files changed, 18 insertions(+) create mode 100644 tests/run-make/optimization-remarks-dir/Makefile create mode 100644 tests/run-make/optimization-remarks-dir/main.rs diff --git a/tests/run-make/optimization-remarks-dir/Makefile b/tests/run-make/optimization-remarks-dir/Makefile new file mode 100644 index 0000000000000..8728f205ec913 --- /dev/null +++ b/tests/run-make/optimization-remarks-dir/Makefile @@ -0,0 +1,12 @@ +include ../tools.mk + +PROFILE_DIR=$(TMPDIR)/profiles + +all: check_inline check_filter + +check_inline: + $(RUSTC) -O main.rs -Cremark=all -Zremark-dir=$(PROFILE_DIR) + cat $(PROFILE_DIR)/*.opt.yaml | $(CGREP) -e "inline" +check_filter: + $(RUSTC) -O main.rs -Cremark=foo -Zremark-dir=$(PROFILE_DIR) + cat $(PROFILE_DIR)/*.opt.yaml | $(CGREP) -e -v "inline" diff --git a/tests/run-make/optimization-remarks-dir/main.rs b/tests/run-make/optimization-remarks-dir/main.rs new file mode 100644 index 0000000000000..8c719626b4f37 --- /dev/null +++ b/tests/run-make/optimization-remarks-dir/main.rs @@ -0,0 +1,6 @@ +#[inline(never)] +fn foo() {} + +fn main() { + foo(); +} From e34ff93c6ba531a759629222cb66f853a4a14942 Mon Sep 17 00:00:00 2001 From: Guilliam Xavier Date: Fri, 30 Jun 2023 16:11:30 +0200 Subject: [PATCH 05/10] std docs: factorize literal in Barrier example --- library/std/src/sync/barrier.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/library/std/src/sync/barrier.rs b/library/std/src/sync/barrier.rs index 11836b7b694b3..e39254aa43491 100644 --- a/library/std/src/sync/barrier.rs +++ b/library/std/src/sync/barrier.rs @@ -13,9 +13,10 @@ use crate::sync::{Condvar, Mutex}; /// use std::sync::{Arc, Barrier}; /// use std::thread; /// -/// let mut handles = Vec::with_capacity(10); -/// let barrier = Arc::new(Barrier::new(10)); -/// for _ in 0..10 { +/// let n = 10; +/// let mut handles = Vec::with_capacity(n); +/// let barrier = Arc::new(Barrier::new(n)); +/// for _ in 0..n { /// let c = Arc::clone(&barrier); /// // The same messages will be printed together. /// // You will NOT see any interleaving. @@ -105,9 +106,10 @@ impl Barrier { /// use std::sync::{Arc, Barrier}; /// use std::thread; /// - /// let mut handles = Vec::with_capacity(10); - /// let barrier = Arc::new(Barrier::new(10)); - /// for _ in 0..10 { + /// let n = 10; + /// let mut handles = Vec::with_capacity(n); + /// let barrier = Arc::new(Barrier::new(n)); + /// for _ in 0..n { /// let c = Arc::clone(&barrier); /// // The same messages will be printed together. /// // You will NOT see any interleaving. From fc61ad393147479f92c8fcdd44a828481a531a00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 1 Jul 2023 10:38:43 +0200 Subject: [PATCH 06/10] Improve error reporting --- compiler/rustc_codegen_llvm/src/back/write.rs | 6 ++-- compiler/rustc_codegen_ssa/messages.ftl | 2 ++ compiler/rustc_codegen_ssa/src/back/write.rs | 10 ++++--- compiler/rustc_codegen_ssa/src/errors.rs | 6 ++++ .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 28 +++++++++++-------- 5 files changed, 34 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index a938bef8397f5..bc2bdf90d75d5 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -290,7 +290,7 @@ impl<'a> DiagnosticHandlers<'a> { handler: &'a Handler, llcx: &'a llvm::Context, module: &ModuleCodegen, - section: CodegenDiagnosticsStage, + stage: CodegenDiagnosticsStage, ) -> Self { let remark_passes_all: bool; let remark_passes: Vec; @@ -312,12 +312,12 @@ impl<'a> DiagnosticHandlers<'a> { .as_ref() // Use the .opt.yaml file suffix, which is supported by LLVM's opt-viewer. .map(|dir| { - let section = match section { + let stage_suffix = match stage { CodegenDiagnosticsStage::Codegen => "codegen", CodegenDiagnosticsStage::Opt => "opt", CodegenDiagnosticsStage::LTO => "lto", }; - dir.join(format!("{}.{section}.opt.yaml", module.name)) + dir.join(format!("{}.{stage_suffix}.opt.yaml", module.name)) }) .and_then(|dir| dir.to_str().and_then(|p| CString::new(p).ok())); diff --git a/compiler/rustc_codegen_ssa/messages.ftl b/compiler/rustc_codegen_ssa/messages.ftl index 5ecb63986fe1f..f73080182bfcd 100644 --- a/compiler/rustc_codegen_ssa/messages.ftl +++ b/compiler/rustc_codegen_ssa/messages.ftl @@ -21,6 +21,8 @@ codegen_ssa_create_temp_dir = couldn't create a temp dir: {$error} codegen_ssa_erroneous_constant = erroneous constant encountered +codegen_ssa_error_creating_remark_dir = failed to create remark directory: {$error} + codegen_ssa_expected_used_symbol = expected `used`, `used(compiler)` or `used(linker)` codegen_ssa_extern_funcs_not_found = some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 6b0df03c1fb9f..4353a87a63750 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -35,6 +35,7 @@ use rustc_span::symbol::sym; use rustc_span::{BytePos, FileName, InnerSpan, Pos, Span}; use rustc_target::spec::{MergeFunctions, SanitizerSet}; +use crate::errors::ErrorCreatingRemarkDir; use std::any::Any; use std::borrow::Cow; use std::fs; @@ -1046,10 +1047,11 @@ fn start_executing_work( let backend_features = tcx.global_backend_features(()); let remark_dir = if let Some(ref dir) = sess.opts.unstable_opts.remark_dir { - // Should this be here? - // Can this conflict with a parallel attempt to create this directory? - fs::create_dir_all(dir).expect("Cannot create remark directory"); - Some(dir.canonicalize().expect("Cannot canonicalize remark directory")) + let result = fs::create_dir_all(dir).and_then(|_| dir.canonicalize()); + match result { + Ok(dir) => Some(dir), + Err(error) => sess.emit_fatal(ErrorCreatingRemarkDir { error }), + } } else { None }; diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index 989274308fb00..056b4abd23533 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -1023,3 +1023,9 @@ pub struct TargetFeatureSafeTrait { #[label(codegen_ssa_label_def)] pub def: Span, } + +#[derive(Diagnostic)] +#[diag(codegen_ssa_error_creating_remark_dir)] +pub struct ErrorCreatingRemarkDir { + pub error: std::io::Error, +} diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index a3f149162545f..553fe6cf087f1 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1877,17 +1877,17 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( void *DiagnosticHandlerContext, bool RemarkAllPasses, std::vector RemarkPasses, + std::unique_ptr RemarksFile, std::unique_ptr RemarkStreamer, - std::unique_ptr LlvmRemarkStreamer, - std::unique_ptr RemarksFile + std::unique_ptr LlvmRemarkStreamer ) : DiagnosticHandlerCallback(DiagnosticHandlerCallback), DiagnosticHandlerContext(DiagnosticHandlerContext), RemarkAllPasses(RemarkAllPasses), RemarkPasses(std::move(RemarkPasses)), + RemarksFile(std::move(RemarksFile)), RemarkStreamer(std::move(RemarkStreamer)), - LlvmRemarkStreamer(std::move(LlvmRemarkStreamer)), - RemarksFile(std::move(RemarksFile)) {} + LlvmRemarkStreamer(std::move(LlvmRemarkStreamer)) {} virtual bool handleDiagnostics(const DiagnosticInfo &DI) override { if (this->LlvmRemarkStreamer) { @@ -1939,11 +1939,11 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( bool RemarkAllPasses = false; std::vector RemarkPasses; - // Since LlvmRemarkStreamer contains a pointer to RemarkStreamer, the ordering of the two + // Since LlvmRemarkStreamer contains a pointer to RemarkStreamer, the ordering of the three // members below is important. + std::unique_ptr RemarksFile; std::unique_ptr RemarkStreamer; std::unique_ptr LlvmRemarkStreamer; - std::unique_ptr RemarksFile; }; std::vector Passes; @@ -1953,9 +1953,9 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( } // We need to hold onto both the streamers and the opened file + std::unique_ptr RemarkFile; std::unique_ptr RemarkStreamer; std::unique_ptr LlvmRemarkStreamer; - std::unique_ptr RemarkFile; if (RemarkFilePath != nullptr) { std::error_code EC; @@ -1964,6 +1964,12 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( EC, llvm::sys::fs::OF_TextWithCRLF ); + if (EC) { + std::string Error = std::string("Cannot create remark file: ") + + toString(errorCodeToError(EC)); + report_fatal_error(Twine(Error)); + } + // Do not delete the file after we gather remarks RemarkFile->keep(); @@ -1974,8 +1980,8 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( ); if (Error E = RemarkSerializer.takeError()) { - // Is this OK? - report_fatal_error("Cannot create remark serializer"); + std::string Error = std::string("Cannot create remark serializer: ") + toString(std::move(E)); + report_fatal_error(Twine(Error)); } RemarkStreamer = std::make_unique(std::move(*RemarkSerializer)); LlvmRemarkStreamer = std::make_unique(*RemarkStreamer); @@ -1986,9 +1992,9 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler( DiagnosticHandlerContext, RemarkAllPasses, Passes, + std::move(RemarkFile), std::move(RemarkStreamer), - std::move(LlvmRemarkStreamer), - std::move(RemarkFile) + std::move(LlvmRemarkStreamer) )); } From c607e206be3acf2a301554dee1472131c3352c51 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 1 Jul 2023 13:18:11 +0200 Subject: [PATCH 07/10] Migrate GUI colors test to original CSS color format --- tests/rustdoc-gui/search-filter.goml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/rustdoc-gui/search-filter.goml b/tests/rustdoc-gui/search-filter.goml index d739471a625a4..9e2855b5e02f9 100644 --- a/tests/rustdoc-gui/search-filter.goml +++ b/tests/rustdoc-gui/search-filter.goml @@ -65,9 +65,9 @@ reload: set-timeout: 2000 wait-for: "#crate-search" assert-css: ("#crate-search", { - "border": "1px solid rgb(224, 224, 224)", - "color": "rgb(0, 0, 0)", - "background-color": "rgb(255, 255, 255)", + "border": "1px solid #e0e0e0", + "color": "black", + "background-color": "white", }) // We now check the dark theme. @@ -75,15 +75,15 @@ click: "#settings-menu" wait-for: "#settings" click: "#theme-dark" wait-for-css: ("#crate-search", { - "border": "1px solid rgb(224, 224, 224)", - "color": "rgb(221, 221, 221)", - "background-color": "rgb(53, 53, 53)", + "border": "1px solid #e0e0e0", + "color": "#ddd", + "background-color": "#353535", }) // And finally we check the ayu theme. click: "#theme-ayu" wait-for-css: ("#crate-search", { - "border": "1px solid rgb(92, 103, 115)", - "color": "rgb(255, 255, 255)", - "background-color": "rgb(15, 20, 25)", + "border": "1px solid #5c6773", + "color": "#fff", + "background-color": "#0f1419", }) From 908574b5e761fca6e0b03ef4e2c8640fcff1218a Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 1 Jul 2023 16:05:55 +0200 Subject: [PATCH 08/10] Fix dropping_copy_types lint from linting in match-arm with side-effects --- .../rustc_lint/src/drop_forget_useless.rs | 2 +- tests/ui/lint/dropping_copy_types.rs | 19 +++++++++++++++++++ tests/ui/lint/dropping_references.rs | 19 +++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/drop_forget_useless.rs b/compiler/rustc_lint/src/drop_forget_useless.rs index 4cea6169dc3d9..467f53d445c15 100644 --- a/compiler/rustc_lint/src/drop_forget_useless.rs +++ b/compiler/rustc_lint/src/drop_forget_useless.rs @@ -194,7 +194,7 @@ fn is_single_call_in_arm<'tcx>( arg: &'tcx Expr<'_>, drop_expr: &'tcx Expr<'_>, ) -> bool { - if matches!(arg.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)) { + if arg.can_have_side_effects() { let parent_node = cx.tcx.hir().find_parent(drop_expr.hir_id); if let Some(Node::Arm(Arm { body, .. })) = &parent_node { return body.hir_id == drop_expr.hir_id; diff --git a/tests/ui/lint/dropping_copy_types.rs b/tests/ui/lint/dropping_copy_types.rs index 2937320e5d833..2412222d6d16d 100644 --- a/tests/ui/lint/dropping_copy_types.rs +++ b/tests/ui/lint/dropping_copy_types.rs @@ -77,3 +77,22 @@ fn issue9482(x: u8) { _ => (), } } + +fn issue112653() { + fn foo() -> Result { + println!("doing foo"); + Ok(0) // result is not always useful, the side-effect matters + } + fn bar() { + println!("doing bar"); + } + + fn stuff() -> Result<(), ()> { + match 42 { + 0 => drop(foo()?), // drop is needed because we only care about side-effects + 1 => bar(), + _ => (), // doing nothing (no side-effects needed here) + } + Ok(()) + } +} diff --git a/tests/ui/lint/dropping_references.rs b/tests/ui/lint/dropping_references.rs index 0d5d484f4517f..bb02cb75a9014 100644 --- a/tests/ui/lint/dropping_references.rs +++ b/tests/ui/lint/dropping_references.rs @@ -97,3 +97,22 @@ fn issue10122(x: u8) { _ => (), } } + +fn issue112653() { + fn foo() -> Result<&'static u8, ()> { + println!("doing foo"); + Ok(&0) // result is not always useful, the side-effect matters + } + fn bar() { + println!("doing bar"); + } + + fn stuff() -> Result<(), ()> { + match 42 { + 0 => drop(foo()?), // drop is needed because we only care about side-effects + 1 => bar(), + _ => (), // doing nothing (no side-effects needed here) + } + Ok(()) + } +} From b9df85f6edeb135dd9abb691862e7e9e30740060 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 1 Jul 2023 21:07:28 +0200 Subject: [PATCH 09/10] Make Rust Analyzer tests faster by compiling less code --- src/bootstrap/test.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 2c1f612e39f52..0c0c07f53874d 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -379,7 +379,9 @@ impl Step for RustAnalyzer { let host = self.host; let compiler = builder.compiler(stage, host); - builder.ensure(tool::RustAnalyzer { compiler, target: self.host }).expect("in-tree tool"); + // We don't need to build the whole Rust Analyzer for the proc-macro-srv test suite, + // but we do need the standard library to be present. + builder.ensure(compile::Std::new(compiler, host)); let workspace_path = "src/tools/rust-analyzer"; // until the whole RA test suite runs on `i686`, we only run From 4fae27135200172b2894915b595f3a2f6ae70c42 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Sat, 1 Jul 2023 19:16:27 -0400 Subject: [PATCH 10/10] Document tracking issue for rustdoc `show-type-layout` --- src/doc/rustdoc/src/unstable-features.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/doc/rustdoc/src/unstable-features.md b/src/doc/rustdoc/src/unstable-features.md index ae180439d23d4..013b93e0129c0 100644 --- a/src/doc/rustdoc/src/unstable-features.md +++ b/src/doc/rustdoc/src/unstable-features.md @@ -310,6 +310,8 @@ the source. ### `--show-type-layout`: add a section to each type's docs describing its memory layout +* Tracking issue: [#113248](https://github.com/rust-lang/rust/issues/113248) + Using this flag looks like this: ```bash