Add instrument_fn attribute#157681
Conversation
|
Some changes occurred in compiler/rustc_passes/src/check_attr.rs cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_hir/src/attrs cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_attr_parsing |
|
r? @oli-obk rustbot has assigned @oli-obk. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
8c64d48 to
8d7140c
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
This attribute enables or disables function instrumentation when using `-Zinstrument-xray` or `-Zinstrument-mcount`. It supports the following usage: `#[instrument_fn = "on|off"]` For XRay, "on" is equivalent to always instrument, and "off" is equivalent to never instrumenting. For mcount, "on" has no effect. "off" disables instrumentation of the function.
8d7140c to
42c43f0
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| fn instrument_function_attr<'ll>( | ||
| cx: &SimpleCx<'ll>, | ||
| sess: &Session, | ||
| instrument_fn: &Option<bool>, |
There was a problem hiding this comment.
| instrument_fn: &Option<bool>, | |
| instrument_fn: Option<bool>, |
| const ON_DUPLICATE: OnDuplicate = OnDuplicate::Error; | ||
|
|
There was a problem hiding this comment.
| const ON_DUPLICATE: OnDuplicate = OnDuplicate::Error; |
Error is the default value.
| "xray-instruction-threshold", | ||
| &threshold.to_string(), | ||
| )); | ||
|
|
| (unstable, import_trait_associated_functions, "1.86.0", Some(134691)), | ||
| /// Allows associated types in inherent impls. | ||
| (incomplete, inherent_associated_types, "1.52.0", Some(8995)), | ||
| /// Enable #[instrument_fn] on function (todo: tracking issue) |
There was a problem hiding this comment.
| /// Enable #[instrument_fn] on function (todo: tracking issue) | |
| /// Enable #[instrument_fn] on functions |
Old todo?
| Some(llvm_mcount_intrinsic) => llvm_mcount_intrinsic.as_ref(), | ||
| None => sess.target.mcount.as_ref(), | ||
| }; | ||
| // #[instrument_fn], the default is on. |
There was a problem hiding this comment.
Isn't it an error to use #[instrument_fn]? There is no default, the value must always be specified.
| let mut instrument = None; | ||
| match args { | ||
| ArgParser::NameValue(nv) => { | ||
| if let Some(option) = nv.value_as_str() |
There was a problem hiding this comment.
this is just stylistic, feel free to ignore.
| if let Some(option) = nv.value_as_str() | |
| if let Some(option @ (sym::on | sym::off)) = nv.value_as_str() |
or my preference:
| if let Some(option) = nv.value_as_str() | |
| match nv.value_as_str(){ | |
| Some(sym::on) => Some(true), | |
| Some(sym::off) => Some(false), | |
| None => error, | |
| } |
| 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 => {} | ||
| } | ||
| if options.never { | ||
|
|
||
| if never { | ||
| attrs.push(llvm::CreateAttrStringValue(cx.llcx, "function-instrument", "xray-never")); | ||
| } | ||
| if always { | ||
| attrs.push(llvm::CreateAttrStringValue(cx.llcx, "function-instrument", "xray-always")); | ||
| } | ||
|
|
There was a problem hiding this comment.
| 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 => {} | |
| } | |
| if options.never { | |
| if never { | |
| attrs.push(llvm::CreateAttrStringValue(cx.llcx, "function-instrument", "xray-never")); | |
| } | |
| if always { | |
| attrs.push(llvm::CreateAttrStringValue(cx.llcx, "function-instrument", "xray-always")); | |
| } | |
| // Apply optional #[instrument_fn] override. | |
| match instrument_fn { | |
| Some(true) => { | |
| attrs.push(llvm::CreateAttrStringValue(cx.llcx, "function-instrument", "xray-always")); | |
| } | |
| Some(false) => { | |
| attrs.push(llvm::CreateAttrStringValue(cx.llcx, "function-instrument", "xray-never")); | |
| } | |
| None => { /* invalid attribute input */ } | |
| } |
|
Reminder, once the PR becomes ready for a review, use |
| InstructionSet(InstructionSetAttr), | ||
|
|
||
| /// Represents `#[instrument_fn]` | ||
| InstrumentFn(Option<bool>), |
There was a problem hiding this comment.
Also a preference thing; I would use an enum for this (perhaps with a Err(guar) variant as well)
enum Instrument {
On,
Off,
}|
☔ The latest upstream changes (presumably #157683) made this pull request unmergeable. Please resolve the merge conflicts. |
This attribute enables or disables function instrumentation when using
-Zinstrument-xrayor-Zinstrument-mcount.It supports the following usage:
#[instrument_fn = "on|off"]For XRay, "on" is equivalent to always instrument, and "off" is equivalent to never instrumenting.
For mcount, "on" has no effect. "off" disables instrumentation of the function.
This is tracked by #157081, and related to the pending rfc rust-lang/rfcs#3917, and earlier as part mcp rust-lang/compiler-team#561