| Message ID | 20250214111936.15168-9-leo.yan@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | perf script: Refactor branch flags for Arm SPE | expand |
On 14/02/2025 11:19 am, Leo Yan wrote: > The new added branch operations and events are filled into record, the > information will be consumed when synthesizing samples. > > Reviewed-by: Ian Rogers <irogers@google.com> > Signed-off-by: Leo Yan <leo.yan@arm.com> > --- > .../util/arm-spe-decoder/arm-spe-decoder.c | 18 ++++++++++++++++++ > .../util/arm-spe-decoder/arm-spe-decoder.h | 10 ++++++++-- > 2 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c > index ba807071d3c1..52bd0a4ea96d 100644 > --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c > +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c > @@ -207,6 +207,18 @@ static int arm_spe_read_record(struct arm_spe_decoder *decoder) > break; > case SPE_OP_PKT_HDR_CLASS_BR_ERET: > decoder->record.op |= ARM_SPE_OP_BRANCH_ERET; > + if (payload & SPE_OP_PKT_COND) > + decoder->record.op |= ARM_SPE_OP_BR_COND; I think this results in memory events being synthesised for these samples because of a bug in arm_spe__synth_data_source(). ARM_SPE_OP_BR_COND overlaps with bits from other packet types like ARM_SPE_OP_LD: if (record->op & ARM_SPE_OP_LD) data_src.mem_op = PERF_MEM_OP_LOAD; arm_spe__synth_data_source() needs to only interpret that as a data source packet if ARM_SPE_OP_LDST is set. This was reported by Mike Williams but you have the privilege of hitting it for real first. > + if (payload & SPE_OP_PKT_INDIRECT_BRANCH) > + decoder->record.op |= ARM_SPE_OP_BR_INDIRECT; > + if (payload & SPE_OP_PKT_GCS) > + decoder->record.op |= ARM_SPE_OP_BR_GCS; > + if (SPE_OP_PKT_CR_BL(payload)) > + decoder->record.op |= ARM_SPE_OP_BR_CR_BL; > + if (SPE_OP_PKT_CR_RET(payload)) > + decoder->record.op |= ARM_SPE_OP_BR_CR_RET; > + if (SPE_OP_PKT_CR_NON_BL_RET(payload)) > + decoder->record.op |= ARM_SPE_OP_BR_CR_NON_BL_RET; > break; > default: > pr_err("Get packet error!\n"); > @@ -238,6 +250,12 @@ static int arm_spe_read_record(struct arm_spe_decoder *decoder) > if (payload & BIT(EV_MISPRED)) > decoder->record.type |= ARM_SPE_BRANCH_MISS; > > + if (payload & BIT(EV_NOT_TAKEN)) > + decoder->record.type |= ARM_SPE_BRANCH_NOT_TAKEN; > + > + if (payload & BIT(EV_TRANSACTIONAL)) > + decoder->record.type |= ARM_SPE_IN_TXN; > + > if (payload & BIT(EV_PARTIAL_PREDICATE)) > decoder->record.type |= ARM_SPE_SVE_PARTIAL_PRED; > > diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h > index 4bcd627e859f..85b688a97436 100644 > --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h > +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h > @@ -24,6 +24,8 @@ enum arm_spe_sample_type { > ARM_SPE_REMOTE_ACCESS = 1 << 7, > ARM_SPE_SVE_PARTIAL_PRED = 1 << 8, > ARM_SPE_SVE_EMPTY_PRED = 1 << 9, > + ARM_SPE_BRANCH_NOT_TAKEN = 1 << 10, > + ARM_SPE_IN_TXN = 1 << 11, > }; > > enum arm_spe_op_type { > @@ -52,8 +54,12 @@ enum arm_spe_op_type { > ARM_SPE_OP_SVE_SG = 1 << 27, > > /* Second level operation type for BRANCH_ERET */ > - ARM_SPE_OP_BR_COND = 1 << 16, > - ARM_SPE_OP_BR_INDIRECT = 1 << 17, > + ARM_SPE_OP_BR_COND = 1 << 16, > + ARM_SPE_OP_BR_INDIRECT = 1 << 17, > + ARM_SPE_OP_BR_GCS = 1 << 18, > + ARM_SPE_OP_BR_CR_BL = 1 << 19, > + ARM_SPE_OP_BR_CR_RET = 1 << 20, > + ARM_SPE_OP_BR_CR_NON_BL_RET = 1 << 21, > }; > > enum arm_spe_common_data_source {
On Mon, Feb 17, 2025 at 04:20:57PM +0000, James Clark wrote: [...] > > diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c > > index ba807071d3c1..52bd0a4ea96d 100644 > > --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c > > +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c > > @@ -207,6 +207,18 @@ static int arm_spe_read_record(struct arm_spe_decoder *decoder) > > break; > > case SPE_OP_PKT_HDR_CLASS_BR_ERET: > > decoder->record.op |= ARM_SPE_OP_BRANCH_ERET; > > + if (payload & SPE_OP_PKT_COND) > > + decoder->record.op |= ARM_SPE_OP_BR_COND; > > I think this results in memory events being synthesised for these > samples because of a bug in arm_spe__synth_data_source(). > ARM_SPE_OP_BR_COND overlaps with bits from other packet types like > ARM_SPE_OP_LD: > > if (record->op & ARM_SPE_OP_LD) > data_src.mem_op = PERF_MEM_OP_LOAD; > > arm_spe__synth_data_source() needs to only interpret that as a data > source packet if ARM_SPE_OP_LDST is set. This was reported by Mike > Williams but you have the privilege of hitting it for real first. Good catching. I have added a new patch 06 to fix the bug in the v3 series. Thanks for hooking up the info. Leo
diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c index ba807071d3c1..52bd0a4ea96d 100644 --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c @@ -207,6 +207,18 @@ static int arm_spe_read_record(struct arm_spe_decoder *decoder) break; case SPE_OP_PKT_HDR_CLASS_BR_ERET: decoder->record.op |= ARM_SPE_OP_BRANCH_ERET; + if (payload & SPE_OP_PKT_COND) + decoder->record.op |= ARM_SPE_OP_BR_COND; + if (payload & SPE_OP_PKT_INDIRECT_BRANCH) + decoder->record.op |= ARM_SPE_OP_BR_INDIRECT; + if (payload & SPE_OP_PKT_GCS) + decoder->record.op |= ARM_SPE_OP_BR_GCS; + if (SPE_OP_PKT_CR_BL(payload)) + decoder->record.op |= ARM_SPE_OP_BR_CR_BL; + if (SPE_OP_PKT_CR_RET(payload)) + decoder->record.op |= ARM_SPE_OP_BR_CR_RET; + if (SPE_OP_PKT_CR_NON_BL_RET(payload)) + decoder->record.op |= ARM_SPE_OP_BR_CR_NON_BL_RET; break; default: pr_err("Get packet error!\n"); @@ -238,6 +250,12 @@ static int arm_spe_read_record(struct arm_spe_decoder *decoder) if (payload & BIT(EV_MISPRED)) decoder->record.type |= ARM_SPE_BRANCH_MISS; + if (payload & BIT(EV_NOT_TAKEN)) + decoder->record.type |= ARM_SPE_BRANCH_NOT_TAKEN; + + if (payload & BIT(EV_TRANSACTIONAL)) + decoder->record.type |= ARM_SPE_IN_TXN; + if (payload & BIT(EV_PARTIAL_PREDICATE)) decoder->record.type |= ARM_SPE_SVE_PARTIAL_PRED; diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h index 4bcd627e859f..85b688a97436 100644 --- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h +++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h @@ -24,6 +24,8 @@ enum arm_spe_sample_type { ARM_SPE_REMOTE_ACCESS = 1 << 7, ARM_SPE_SVE_PARTIAL_PRED = 1 << 8, ARM_SPE_SVE_EMPTY_PRED = 1 << 9, + ARM_SPE_BRANCH_NOT_TAKEN = 1 << 10, + ARM_SPE_IN_TXN = 1 << 11, }; enum arm_spe_op_type { @@ -52,8 +54,12 @@ enum arm_spe_op_type { ARM_SPE_OP_SVE_SG = 1 << 27, /* Second level operation type for BRANCH_ERET */ - ARM_SPE_OP_BR_COND = 1 << 16, - ARM_SPE_OP_BR_INDIRECT = 1 << 17, + ARM_SPE_OP_BR_COND = 1 << 16, + ARM_SPE_OP_BR_INDIRECT = 1 << 17, + ARM_SPE_OP_BR_GCS = 1 << 18, + ARM_SPE_OP_BR_CR_BL = 1 << 19, + ARM_SPE_OP_BR_CR_RET = 1 << 20, + ARM_SPE_OP_BR_CR_NON_BL_RET = 1 << 21, }; enum arm_spe_common_data_source {