L2l2transfers new

[predutta]

Problem solved by the commit
To measure L2-L2 (memtile-memtile) transfer efficiency (running vs stalled time on inter-stamp halo paths).

How problem was solved, alternative solutions (if any) and why they were rejected
This PR adds L2-L2 memtile dtrace support to the VE2 AIE dtrace plugin so we can measure L2-L2 transfer efficiency (running vs stalled time on inter-stamp halo paths). Conventional AIE trace is not viable on memtile rows in these overlay designs because trace stream connections are already consumed, while memtile performance counters keep accumulating once programmed. We use the existing aie_dtrace flow to configure those counters in the CT begin block and sample them at kernel layer boundaries via read_reg jprobes, without extra trace plumbing. L2-L2 counters are appended to the same per-run CT as shim bandwidth metrics, with metadata merged into per-stamp groups for post-processing.

Configuration uses two separate sources (do not mix):

1. xrt.ini — when Debug.profiling_runtime_config does not supply mem-tile L2-L2 settings, set both keys under [AIE_dtrace_settings]:

[AIE_dtrace_settings]
tile_based_memory_tile_metrics=all:input_ports
memory_tile_input_ports={1,1:2},{5,1:1},{5,1:2},{9,1:1},{9,1:2},{13,1:1},{13,1:2},{17,1:1},{17,1:2},{21,1:1}

2. control_instrumentation blob — when Debug.profiling_runtime_config includes control_instrumentation.mem_tile or control_instrumentation.memory_tile_input_ports, the full mem-tile L2-L2 config must come from the blob (both fields). xrt.ini mem-tile keys are not used as fallback:

{
"control_instrumentation": {
"mem_tile": "input_ports",
"memory_tile_input_ports": "{1,1:2},{5,1:1},{5,1:2},{9,1:1},{9,1:2},{13,1:1},{13,1:2},{17,1:1},{17,1:2},{21,1:1}"
}
}

mem_tile must be "input_ports" to enable L2-L2. memory_tile_input_ports is a {column,row:port} list; column is partition-relative (0 = partition start_col; same model as shim bandwidth). Row is for readability only; counters are programmed on memtile row 1.

What has been tested and how, request additional testing if necessary
Tested on Telluride with the baseline 6x4x4 overlay (6 stamps) that includes L2-L2 connections, using xrt.ini configuration as above.

[Copilot]

Pull request overview

Adds VE2 AIE dtrace support for memtile L2–L2 transfer efficiency measurement by configuring memtile performance counters in the CT begin block and sampling them at kernel-layer boundaries, enabling analysis of running vs stalled time on inter-stamp halo paths without consuming trace stream resources.

Changes:

• Enable/advertise a new memtile metric set (l2_l2_transfer) in CT generation and logging.
• Add CT-writer support to append memtile perf-counter configuration and metadata for L2–L2 counters based on AIE_dtrace_settings.l2_l2_design_points.
• Extend metadata parsing to detect when L2–L2 is enabled via tile_based_memory_tile_metrics (and via control instrumentation blob) and warn when design points are missing/invalid.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
profile/plugin/aie_dtrace/ve2/aie_dtrace_ve2.cpp	Keeps CT generation enabled when only L2–L2 memtile metrics are selected; improves generated-CT logging.
profile/plugin/aie_dtrace/ve2/aie_dtrace_ct_writer.h	Adds L2–L2 CT-writer APIs and memtile perf-control offsets/event constants.
profile/plugin/aie_dtrace/ve2/aie_dtrace_ct_writer.cpp	Appends L2–L2 counters/writes into CT and annotates generated CT output; adds memtile perf-counter configuration generation.
profile/plugin/aie_dtrace/util/aie_dtrace_util.h	Introduces L2–L2 design-point and counter-point data structures and parsing APIs.
profile/plugin/aie_dtrace/util/aie_dtrace_util.cpp	Implements parsing of l2_l2_design_points and mapping to running/stalled counter pairs.
profile/plugin/aie_dtrace/aie_dtrace_metadata.h	Adds l2L2TransferEnabled flag + accessor.
profile/plugin/aie_dtrace/aie_dtrace_metadata.cpp	Detects/enables L2–L2 based on ini/blob settings and validates presence of design points.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

There is a confirmed build-breaking issue (#// SPDX-License-Identifier preprocessor directive) and a concrete configuration/doc mismatch that can prevent L2–L2 enablement as described.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

profile/plugin/aie_dtrace/aie_dtrace_metadata.cpp:126

• Related to the same config/doc mismatch: when enabling from the runtime-config blob, mem_tile must currently be exactly input_ports, so a blob using l2_l2_transfer (as described in the PR text) would not enable L2-L2. If you accept the alias in settingsRequestL2L2Transfer, it should also be accepted here.

      if (memTileFieldFromBlob && *ci.mem_tile == INPUT_PORTS_METRIC_SET) {

• Files reviewed: 9/9 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It adds hardware-specific memtile performance-counter programming and CT-generation behavior that should be validated by a human reviewer with platform knowledge.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

profile/plugin/aie_dtrace/ve2/aie_dtrace_ct_writer.cpp:13

• core/common/config_reader.h is included here but not referenced anywhere in this translation unit, which can trigger unused-include linting and adds unnecessary compile overhead. Please remove it.
  profile/plugin/aie_dtrace/ve2/aie_dtrace_ct_writer.cpp:1069
• The CT header comment refers to tile_based_memory_tile_metrics generically, but this plugin reads it from [AIE_dtrace_settings] (not [AIE_trace_settings]). Qualifying the setting name avoids confusion with similarly named trace settings.

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It introduces new hardware-counter configuration and CT generation paths whose correctness depends on device/overlay-specific behavior and warrants a final human validation pass.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There is at least one confirmed correctness issue in the new L2–L2 CT path (missing hwctx/exception safety) that can lead to crashes when L2–L2 is enabled.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

profile/plugin/aie_dtrace/ve2/aie_dtrace_ct_writer.cpp:15

• core/common/config_reader.h is included but not referenced in this translation unit; it can be removed to reduce build dependencies and avoid -Wunused-include warnings in some toolchains.

#include "core/common/config_reader.h"
#include "core/common/message.h"
#include "xdp/profile/plugin/vp_base/profiling_runtime_config.h"

• Files reviewed: 9/9 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

AieDtraceMetadata::isConfigured() does not account for L2-L2-only configurations, which can break config_one_partition=true enforcement when memtile L2-L2 is the only enabled metric.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

profile/plugin/aie_dtrace/aie_dtrace_metadata.h:67

• isConfigured() currently ignores l2L2TransferEnabled, so a configuration that enables only memtile L2-L2 counters (with shim/core metrics disabled) will be treated as “not configured”. This breaks config_one_partition=true bookkeeping in AieDtracePlugin (it only records a partition as configured when metadata->isConfigured() is true), allowing subsequent partitions to be configured unexpectedly.

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are confirmed behavioral/documentation inconsistencies that can lead to surprising configuration outcomes and should be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It introduces new hardware register programming for memtile performance counters and new configuration-source precedence behavior that typically requires broader hardware/integration validation to sign off safely.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It introduces new hardware counter programming paths and configuration-resolution logic that are difficult to fully validate for correctness and platform coverage via static review alone.

Review details

Suppressed comments (1)

profile/plugin/vp_base/profiling_runtime_config.h:121

• This comment says that if control_instrumentation "carries" mem_tile or memory_tile_input_ports then ports come only from the blob, but the implementation only treats non-empty values as authoritative (empty-string fields fall back to xrt.ini). Clarify this to match the actual resolution logic and avoid configuration confusion.

  // When control_instrumentation carries mem_tile or memory_tile_input_ports,
  // ports come only from the blob (requires mem_tile "input_ports"). Otherwise
  // AIE_dtrace_settings.memory_tile_input_ports from xrt.ini is used.

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Include L2-L2 enablement in isConfigured() and reject partially matched design-point lists.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

profile/plugin/aie_dtrace/aie_dtrace_metadata.h:67

• The new L2-L2 flag is not included in isConfigured(), even though AieDtracePlugin::updateAIEDtraceDevice() uses that result to set configuredOnePartition. With config_one_partition=true and interface/core metrics disabled, an L2-only configuration therefore leaves isConfigured() false and allows every partition to be configured instead of only the first one. Include l2L2TransferEnabled in the return expression.

profile/plugin/aie_dtrace/util/aie_dtrace_util.cpp:70

• The regex iterator accepts valid-looking substrings and silently ignores the rest of the value. For example, {1,1:2},broken produces one point, passes the metadata check for designPoints.empty(), and enables incomplete instrumentation without warning, so the reported L2-L2 efficiency can omit requested paths. Validate the complete list (including separators) or reject the configuration when any fragment is unmatched.

    static const std::regex pointRegex(R"(\{\s*(\d+)\s*,\s*(\d+)\s*:\s*(\d+)\s*\})");
    const auto begin = std::sregex_iterator(spec.begin(), spec.end(), pointRegex);
    const auto end = std::sregex_iterator();
    for (auto it = begin; it != end; ++it) {

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

A critical compilation issue and a moderate L2-L2 configuration issue remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

profile/plugin/aie_dtrace/aie_dtrace_metadata.h:67

• The new L2-L2-only path can leave both shim/core metric maps empty (for example when interface/core metrics are off), but AieDtraceMetadata::isConfigured() still returns false unless those maps contain entries. aie_dtrace_plugin.cpp:148-149 uses that method to enforce config_one_partition, so with config_one_partition=true each additional partition can still be configured when L2-L2 is the only enabled family. Include l2L2TransferEnabled in the configured check.

    bool isL2L2Enabled() const { return l2L2TransferEnabled; }

• Files reviewed: 9/9 changed files
• Comments generated: 1
• Review effort level: Lite