Refactor -Zinstrument-mcount, -Zinstrument-fentry, -Zinstrument-xray

Convert these into `-Zinstrument-function={none|fentry|xray}`.

`-Zinstrument-xray` is split into `-Zinstrument-function=xray` and
`-Zinstrument-xray-opts=...`.

Also, rename `-Zinstrument-mcount-opts` to `-Zinstrument-fentry-opts` to
reflect it is only usable with fentry. In theory, it could work with
both (as it does on gcc), in practice the llvm support is limited to
s390x/fentry as of llvm 22.

Author: pmur

compiler/rustc_codegen_llvm/src/attributes.rs

@@ -6,7 +6,9 @@ use rustc_middle::middle::codegen_fn_attrs::{
     CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry, SanitizerFnAttrs, TargetFeature,
 };
 use rustc_middle::ty::{self, TyCtxt};
-use rustc_session::config::{BranchProtection, FunctionReturn, OptLevel, PAuthKey, PacRet};
+use rustc_session::config::{
+    BranchProtection, FunctionReturn, InstrumentFunction, OptLevel, PAuthKey, PacRet,
+};
 use rustc_span::sym;
 use rustc_symbol_mangling::mangle_internal_symbol;
 use rustc_target::spec::{Arch, FramePointer, SanitizerSet, StackProbeType, StackProtector};
@@ -175,7 +177,7 @@ pub(crate) fn frame_pointer_type_attr<'ll>(
     let opts = &sess.opts;
     // "mcount" function relies on stack pointer.
     // See <https://sourceware.org/binutils/docs/gprof/Implementation.html>.
-    if opts.unstable_opts.instrument_mcount {
+    if opts.unstable_opts.instrument_function == InstrumentFunction::Mcount {
         fp.ratchet(FramePointer::Always);
     }
     fp.ratchet(opts.cg.force_frame_pointers);
@@ -204,83 +206,101 @@ fn instrument_function_attr<'ll>(
     instrument_fn: &Option<bool>,
 ) -> SmallVec<[&'ll Attribute; 4]> {
     let mut attrs = SmallVec::new();
-    if sess.opts.unstable_opts.instrument_mcount || sess.opts.unstable_opts.instrument_fentry {
-        // Similar to `clang -pg` behavior. Handled by the
-        // `post-inline-ee-instrument` LLVM pass.
-
-        // #[instrument_fn], the default is on.
-        let instrument_entry = instrument_fn.unwrap_or_else(|| true);
-
-        if instrument_entry && sess.opts.unstable_opts.instrument_mcount {
-            // The function name varies on platforms.
-            // See test/CodeGen/mcount.c in clang.
-            let mcount_name = match &sess.target.llvm_mcount_intrinsic {
-                Some(llvm_mcount_intrinsic) => llvm_mcount_intrinsic.as_ref(),
-                None => sess.target.mcount.as_ref(),
-            };
+    match sess.opts.unstable_opts.instrument_function {
+        InstrumentFunction::Fentry => {
+            // Similar to `clang -pg -mfentry` behavior.
 
-            attrs.push(llvm::CreateAttrStringValue(
-                cx.llcx,
-                "instrument-function-entry-inlined",
-                mcount_name,
-            ));
-        }
+            // #[instrument_fn], the default is on.
+            let instrument_entry = instrument_fn.unwrap_or_else(|| true);
 
-        if instrument_entry && sess.opts.unstable_opts.instrument_fentry {
-            attrs.push(llvm::CreateAttrStringValue(cx.llcx, "fentry-call", "true"));
-        }
+            if instrument_entry {
+                attrs.push(llvm::CreateAttrStringValue(cx.llcx, "fentry-call", "true"));
 
-        if sess.opts.unstable_opts.instrument_mcount_opts.no_call {
-            attrs.push(llvm::CreateAttrString(cx.llcx, "mnop-mcount"));
+                if sess.opts.unstable_opts.instrument_fentry_opts.no_call {
+                    attrs.push(llvm::CreateAttrString(cx.llcx, "mnop-mcount"));
+                }
+                if sess.opts.unstable_opts.instrument_fentry_opts.record {
+                    attrs.push(llvm::CreateAttrString(cx.llcx, "mrecord-mcount"));
+                }
+            }
         }
-        if sess.opts.unstable_opts.instrument_mcount_opts.record {
-            attrs.push(llvm::CreateAttrString(cx.llcx, "mrecord-mcount"));
+        InstrumentFunction::Mcount => {
+            // Similar to `clang -pg` behavior. Handled by the
+            // `post-inline-ee-instrument` LLVM pass.
+
+            // #[instrument_fn], the default is on.
+            let instrument_entry = instrument_fn.unwrap_or_else(|| true);
+
+            if instrument_entry {
+                // The function name varies on platforms.
+                // See test/CodeGen/mcount.c in clang.
+                let mcount_name = match &sess.target.llvm_mcount_intrinsic {
+                    Some(llvm_mcount_intrinsic) => llvm_mcount_intrinsic.as_ref(),
+                    None => sess.target.mcount.as_ref(),
+                };
+
+                attrs.push(llvm::CreateAttrStringValue(
+                    cx.llcx,
+                    "instrument-function-entry-inlined",
+                    mcount_name,
+                ));
+            }
         }
-    }
-    if let Some(options) = &sess.opts.unstable_opts.instrument_xray {
-        // XRay instrumentation is similar to __cyg_profile_func_{enter,exit}.
-        // Function prologue and epilogue are instrumented with NOP sleds,
-        // a runtime library later replaces them with detours into tracing code.
-
-        let mut never = options.never;
-        let mut always = options.always;
+        InstrumentFunction::XRay => {
+            // XRay instrumentation is similar to __cyg_profile_func_{enter,exit}.
+            // Function prologue and epilogue are instrumented with NOP sleds,
+            // a runtime library later replaces them with detours into tracing code.
+            let options = &sess.opts.unstable_opts.instrument_xray_opts;
+
+            let mut never = options.never;
+            let mut always = options.always;
+
+            // Apply optional #[instrument_fn] override.
+            match instrument_fn {
+                Some(true) => {
+                    always = true;
+                }
+                Some(false) => {
+                    never = true;
+                }
+                None => {}
+            }
 
-        // Apply optional #[instrument_fn] override.
-        match instrument_fn {
-            Some(true) => {
-                always = true;
+            if never {
+                attrs.push(llvm::CreateAttrStringValue(
+                    cx.llcx,
+                    "function-instrument",
+                    "xray-never",
+                ));
             }
-            Some(false) => {
-                never = true;
+            if always {
+                attrs.push(llvm::CreateAttrStringValue(
+                    cx.llcx,
+                    "function-instrument",
+                    "xray-always",
+                ));
             }
-            None => {}
-        }
 
-        if never {
-            attrs.push(llvm::CreateAttrStringValue(cx.llcx, "function-instrument", "xray-never"));
-        }
-        if always {
-            attrs.push(llvm::CreateAttrStringValue(cx.llcx, "function-instrument", "xray-always"));
-        }
-
-        if options.ignore_loops {
-            attrs.push(llvm::CreateAttrString(cx.llcx, "xray-ignore-loops"));
-        }
-        // LLVM will not choose the default for us, but rather requires specific
-        // threshold in absence of "xray-always". Use the same default as Clang.
-        let threshold = options.instruction_threshold.unwrap_or(200);
-        attrs.push(llvm::CreateAttrStringValue(
-            cx.llcx,
-            "xray-instruction-threshold",
-            &threshold.to_string(),
-        ));
+            if options.ignore_loops {
+                attrs.push(llvm::CreateAttrString(cx.llcx, "xray-ignore-loops"));
+            }
+            // LLVM will not choose the default for us, but rather requires specific
+            // threshold in absence of "xray-always". Use the same default as Clang.
+            let threshold = options.instruction_threshold.unwrap_or(200);
+            attrs.push(llvm::CreateAttrStringValue(
+                cx.llcx,
+                "xray-instruction-threshold",
+                &threshold.to_string(),
+            ));
 
-        if options.skip_entry {
-            attrs.push(llvm::CreateAttrString(cx.llcx, "xray-skip-entry"));
-        }
-        if options.skip_exit {
-            attrs.push(llvm::CreateAttrString(cx.llcx, "xray-skip-exit"));
+            if options.skip_entry {
+                attrs.push(llvm::CreateAttrString(cx.llcx, "xray-skip-entry"));
+            }
+            if options.skip_exit {
+                attrs.push(llvm::CreateAttrString(cx.llcx, "xray-skip-exit"));
+            }
         }
+        InstrumentFunction::No => {}
     }
     attrs
 }

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -35,8 +35,8 @@ use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
 use rustc_middle::middle::dependency_format::Linkage;
 use rustc_middle::middle::exported_symbols::SymbolExportKind;
 use rustc_session::config::{
-    self, CFGuard, CrateType, DebugInfo, LinkerFeaturesCli, OutFileName, OutputFilenames,
-    OutputType, PrintKind, SplitDwarfKind, Strip,
+    self, CFGuard, CrateType, DebugInfo, InstrumentFunction, LinkerFeaturesCli, OutFileName,
+    OutputFilenames, OutputType, PrintKind, SplitDwarfKind, Strip,
 };
 use rustc_session::lint::builtin::LINKER_MESSAGES;
 use rustc_session::output::{check_file_is_writeable, invalid_output_for_target, out_filename};
@@ -2767,8 +2767,11 @@ fn add_order_independent_options(
         cmd.pgo_gen();
     }
 
-    if sess.opts.unstable_opts.instrument_mcount {
-        cmd.enable_profiling();
+    match sess.opts.unstable_opts.instrument_function {
+        InstrumentFunction::Mcount | InstrumentFunction::Fentry => {
+            cmd.enable_profiling();
+        }
+        _ => {}
     }
 
     if sess.opts.cg.control_flow_guard != CFGuard::Disabled {

compiler/rustc_interface/src/tests.rs

@@ -13,11 +13,12 @@ use rustc_session::config::{
     AnnotateMoves, AutoDiff, BranchProtection, CFGuard, Cfg, CoverageLevel, CoverageOptions,
     DebugInfo, DumpMonoStatsFormat, ErrorOutputType, ExternEntry, ExternLocation, Externs,
     FmtDebug, FunctionReturn, IncrementalStateAssertion, InliningThreshold, Input,
-    InstrumentCoverage, InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli,
-    MirIncludeSpans, NextSolverConfig, Offload, Options, OutFileName, OutputType, OutputTypes,
-    PAuthKey, PacRet, Passes, PatchableFunctionEntry, Polonius, ProcMacroExecutionStrategy, Strip,
-    SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, build_configuration,
-    build_session_options, rustc_optgroups,
+    InstrumentCoverage, InstrumentFunction, InstrumentMcountOpts, InstrumentXRayOpts,
+    LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli, MirIncludeSpans, NextSolverConfig,
+    Offload, Options, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet, Passes,
+    PatchableFunctionEntry, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath,
+    SymbolManglingVersion, WasiExecModel, build_configuration, build_session_options,
+    rustc_optgroups,
 };
 use rustc_session::lint::Level;
 use rustc_session::search_paths::SearchPath;
@@ -810,8 +811,12 @@ fn test_unstable_options_tracking_hash() {
     tracked!(inline_mir, Some(true));
     tracked!(inline_mir_hint_threshold, Some(123));
     tracked!(inline_mir_threshold, Some(123));
-    tracked!(instrument_mcount, true);
-    tracked!(instrument_xray, Some(InstrumentXRay::default()));
+    tracked!(instrument_fentry_opts, InstrumentMcountOpts { no_call: true, record: true });
+    tracked!(instrument_function, InstrumentFunction::Fentry);
+    tracked!(
+        instrument_xray_opts,
+        InstrumentXRayOpts { always: true, ..InstrumentXRayOpts::default() }
+    );
     tracked!(link_directives, false);
     tracked!(link_only, true);
     tracked!(lint_llvm_ir, true);

compiler/rustc_session/src/config.rs

@@ -151,6 +151,19 @@ pub enum InstrumentCoverage {
     Yes,
 }
 
+/// The different settings that the `-Z instrument-function` flag can have.
+#[derive(Clone, Copy, PartialEq, Hash, Debug)]
+pub enum InstrumentFunction {
+    /// No instrumentation requested, or `-Z instrument-function=none`
+    No,
+    /// `-Z instrument-function=fentry`
+    Fentry,
+    /// `-Z instrument-function=mcount`
+    Mcount,
+    /// `-Z instrument-function=xray`
+    XRay,
+}
+
 /// Individual flag values controlled by `-Zcoverage-options`.
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Default)]
 pub struct CoverageOptions {
@@ -256,22 +269,22 @@ pub struct InstrumentMcountOpts {
     pub record: bool,
 }
 
-/// Settings for `-Z instrument-xray` flag.
+/// Settings for `-Z instrument-xray-opts` flag.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)]
-pub struct InstrumentXRay {
-    /// `-Z instrument-xray=always`, force instrumentation
+pub struct InstrumentXRayOpts {
+    /// `-Z instrument-xray-opts=always`, force instrumentation
     pub always: bool,
-    /// `-Z instrument-xray=never`, disable instrumentation
+    /// `-Z instrument-xray-opts=never`, disable instrumentation
     pub never: bool,
-    /// `-Z instrument-xray=ignore-loops`, ignore presence of loops,
+    /// `-Z instrument-xray-opts=ignore-loops`, ignore presence of loops,
     /// instrument functions based only on instruction count
     pub ignore_loops: bool,
-    /// `-Z instrument-xray=instruction-threshold=N`, explicitly set instruction threshold
+    /// `-Z instrument-xray-opts=instruction-threshold=N`, explicitly set instruction threshold
     /// for instrumentation, or `None` to use compiler's default
     pub instruction_threshold: Option<usize>,
-    /// `-Z instrument-xray=skip-entry`, do not instrument function entry
+    /// `-Z instrument-xray-opts=skip-entry`, do not instrument function entry
     pub skip_entry: bool,
-    /// `-Z instrument-xray=skip-exit`, do not instrument function exit
+    /// `-Z instrument-xray-opts=skip-exit`, do not instrument function exit
     pub skip_exit: bool,
 }
 
@@ -3066,11 +3079,11 @@ pub(crate) mod dep_tracking {
     use super::{
         AnnotateMoves, AutoDiff, BranchProtection, CFGuard, CFProtection, CoverageOptions,
         CrateType, DebugInfo, DebugInfoCompression, ErrorOutputType, FmtDebug, FunctionReturn,
-        InliningThreshold, InstrumentCoverage, InstrumentMcountOpts, InstrumentXRay,
-        LinkerPluginLto, LocationDetail, LtoCli, MirStripDebugInfo, NextSolverConfig, Offload,
-        OptLevel, OutFileName, OutputType, OutputTypes, PatchableFunctionEntry, Polonius,
-        ResolveDocLinks, SourceFileHashAlgorithm, SplitDwarfKind, SwitchWithOptPath,
-        SymbolManglingVersion, WasiExecModel,
+        InliningThreshold, InstrumentCoverage, InstrumentFunction, InstrumentMcountOpts,
+        InstrumentXRayOpts, LinkerPluginLto, LocationDetail, LtoCli, MirStripDebugInfo,
+        NextSolverConfig, Offload, OptLevel, OutFileName, OutputType, OutputTypes,
+        PatchableFunctionEntry, Polonius, ResolveDocLinks, SourceFileHashAlgorithm, SplitDwarfKind,
+        SwitchWithOptPath, SymbolManglingVersion, WasiExecModel,
     };
     use crate::lint;
     use crate::utils::NativeLib;
@@ -3131,9 +3144,10 @@ pub(crate) mod dep_tracking {
         CodeModel,
         TlsModel,
         InstrumentCoverage,
+        InstrumentFunction,
         CoverageOptions,
         InstrumentMcountOpts,
-        InstrumentXRay,
+        InstrumentXRayOpts,
         CrateType,
         MergeFunctions,
         OnBrokenPipe,