opcode and patching schema for pl ip to transfer wts by HimanshuChoudhary-Xilinx · Pull Request #315 · Xilinx/aiebu

HimanshuChoudhary-Xilinx · 2026-07-01T12:09:04Z

Problem solved by the commit

https://amd.atlassian.net/wiki/spaces/AIE/pages/1744115100/Weight+Prefetch+via+PL+IP+and+PLIO+Feature+Specification
Support new opcode and patching schema to support PL IP
Opcode:

UC_DMA_MASK_POLL_EXT
APPLY_OFFSET_PL

Patching schema:

pl_ddr_64

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

How problem was solved, alternative solutions (if any) and why they were rejected

Risks (if any) associated the changes in the commit

What has been tested and how, request additional testing if necessary

Documentation impact (if any)

TEST

Added PLIP to test all 3.

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

github-actions

clang-tidy made some suggestions

github-actions · 2026-07-01T12:14:21Z


+FORCE_INLINE_FOR_RELEASE_ONLY static inline unsigned int control_dispatch_uc_dma_mask_poll_ext(const uint8_t *pc)
+{
+  return control_op_uc_dma_mask_poll_ext(


warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]

return control_op_uc_dma_mask_poll_ext( ^

github-actions · 2026-07-01T12:14:21Z

+{
+  return control_op_uc_dma_mask_poll_ext(
+    pc,
+    /* addr_hi (const) */ *(uint32_t *)(&pc[4]),


warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

/* addr_hi (const) */ *(uint32_t *)(&pc[4]), ^

github-actions · 2026-07-01T12:14:21Z

+  return control_op_uc_dma_mask_poll_ext(
+    pc,
+    /* addr_hi (const) */ *(uint32_t *)(&pc[4]),
+    /* addr_lo (const) */ *(uint32_t *)(&pc[8]),


warning: 8 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

/* addr_lo (const) */ *(uint32_t *)(&pc[8]), ^

github-actions · 2026-07-01T12:14:21Z

+  return control_op_uc_dma_mask_poll_ext(
+    pc,
+    /* addr_hi (const) */ *(uint32_t *)(&pc[4]),
+    /* addr_lo (const) */ *(uint32_t *)(&pc[8]),


warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

/* addr_lo (const) */ *(uint32_t *)(&pc[8]), ^

github-actions · 2026-07-01T12:14:21Z

+    pc,
+    /* addr_hi (const) */ *(uint32_t *)(&pc[4]),
+    /* addr_lo (const) */ *(uint32_t *)(&pc[8]),
+    /* mask (const) */ *(uint32_t *)(&pc[12]),


warning: 12 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

/* mask (const) */ *(uint32_t *)(&pc[12]), ^

github-actions · 2026-07-01T12:14:22Z

+    pc,
+    /* table_ptr (const) */ *(uint16_t *)(&pc[2]),
+    /* num_entries (const) */ *(uint16_t *)(&pc[4]),
+    /* buffer_id (const) */ *(uint16_t *)(&pc[6])


warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

/* buffer_id (const) */ *(uint16_t *)(&pc[6]) ^

github-actions · 2026-07-01T12:14:22Z

 #define ISA_OPCODE_SLEEP 0x1d
 #define ISA_OPCODE_SAVE_REGISTER 0x1e
 #define ISA_OPCODE_REL_ACQ_SYNC 0x21
+#define ISA_OPCODE_UC_DMA_MASK_POLL_EXT 0x22


warning: macro 'ISA_OPCODE_UC_DMA_MASK_POLL_EXT' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]

#define ISA_OPCODE_UC_DMA_MASK_POLL_EXT 0x22 ^

github-actions · 2026-07-01T12:14:22Z

 #define ISA_OPCODE_SAVE_REGISTER 0x1e
 #define ISA_OPCODE_REL_ACQ_SYNC 0x21
+#define ISA_OPCODE_UC_DMA_MASK_POLL_EXT 0x22
+#define ISA_OPCODE_APPLY_OFFSET_PL 0x23


warning: macro 'ISA_OPCODE_APPLY_OFFSET_PL' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]

#define ISA_OPCODE_APPLY_OFFSET_PL 0x23 ^

github-actions · 2026-07-01T12:14:22Z

 #define ISA_OPSIZE_SLEEP 0x08
 #define ISA_OPSIZE_SAVE_REGISTER 0x0c
 #define ISA_OPSIZE_REL_ACQ_SYNC 0x0c
+#define ISA_OPSIZE_UC_DMA_MASK_POLL_EXT 0x14


warning: macro 'ISA_OPSIZE_UC_DMA_MASK_POLL_EXT' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]

#define ISA_OPSIZE_UC_DMA_MASK_POLL_EXT 0x14 ^

github-actions · 2026-07-01T12:14:22Z

 #define ISA_OPSIZE_SAVE_REGISTER 0x0c
 #define ISA_OPSIZE_REL_ACQ_SYNC 0x0c
+#define ISA_OPSIZE_UC_DMA_MASK_POLL_EXT 0x14
+#define ISA_OPSIZE_APPLY_OFFSET_PL 0x08


warning: macro 'ISA_OPSIZE_APPLY_OFFSET_PL' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]

#define ISA_OPSIZE_APPLY_OFFSET_PL 0x08 ^

Copilot

Pull request overview

This PR extends the AIE2PS control-code toolchain to support PL IP weight-transfer flows by introducing new ISA operations and a new patching schema, along with an end-to-end asm/ELF round-trip test.

Changes:

Add new AIE2PS ISA operations: UC_DMA_MASK_POLL_EXT and APPLY_OFFSET_PL (spec, stubs, docs, dispatcher defines).
Add new relocation/patch schema (pl_ddr_64) and implement read/write + opcode transformation support in the analyzer/transform path.
Add a new plip test suite that assembles ctrlcode, validates the ELF md5, and checks ELF→legacy output.

Reviewed changes

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

Show a summary per file

File	Description
test/aie2ps-ctrlcode/plip/gold.md5	Adds expected md5 for generated ELF artifact.
test/aie2ps-ctrlcode/plip/ctrlcode.txt	Adds golden legacy text output for round-trip validation.
test/aie2ps-ctrlcode/plip/ctrlcode.asm	Adds a ctrlcode program exercising `APPLY_OFFSET_PL` + `UC_DMA_MASK_POLL_EXT`.
test/aie2ps-ctrlcode/plip/CMakeLists.txt	Adds CTest targets for asm/md5/elf-to-legacy/compare.
test/aie2ps-ctrlcode/CMakeLists.txt	Wires the new `plip` test subdirectory into the test suite.
src/tree-sitter/tree-sitter-aie4-ctrlcode/grammar.js	Adds parsing for `UC_DMA_MASK_POLL_EXT` (but currently misses `APPLY_OFFSET_PL`).
src/cpp/ops/ops.cpp	Emits per-`wts_params` `pl_ddr_64` relocations when assembling `apply_offset_pl`.
src/cpp/common/symbol.h	Adds `pl_ddr_64` to `patch_schema` (currently renumbers `unknown`).
src/cpp/common/assembler_state.h	Enables opcode optimization flagging for `apply_offset_pl`.
src/cpp/common/assembler_state.cpp	Tracks label/entries metadata for `apply_offset_pl` similarly to `apply_offset_57`.
src/cpp/analyzer/transform_manager.h	Declares `pl_ddr_64` read/write helpers.
src/cpp/analyzer/transform_manager.cpp	Implements `pl_ddr_64` read/write and handles `OPCODE_APPLY_OFFSET_PL` updates.
specification/aie2ps/isa.h	Adds opcodes + assembler/disassembler entries for the two new operations.
specification/aie2ps/isa-spec.yaml	Documents the two new operations in the ISA spec source.
specification/aie2ps/isa-spec.md	Updates rendered markdown ISA spec with the two new operations.
specification/aie2ps/isa-spec.html	Updates rendered HTML ISA spec with the two new operations.
specification/aie2ps/isa_stubs.h	Adds opcode + opsize defines for the two new operations.
specification/aie2ps/isa_defines.h	Adds dispatcher stubs and macro cases for the two new operations.

Comments suppressed due to low confidence (1)

src/tree-sitter/tree-sitter-aie4-ctrlcode/grammar.js:146

APPLY_OFFSET_PL (introduced in this PR) won't parse with the current apply_offset_statement rule, which only matches APPLY_OFFSET_\d+. This breaks tree-sitter parsing/highlighting for new ctrlcode that uses APPLY_OFFSET_PL (e.g., the added plip test).

    apply_offset_statement: $ => seq(
      token(seq('APPLY_OFFSET_', /\d+/)),
      $.address,
      ',',
      $.decint,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

    control_packet_57 = 7,
    address_64 = 8,
    control_packet_57_aie4 = 9,
-    unknown = 10,
+    pl_ddr_64 = 10,
+    unknown = 11,


larry9523 · 2026-07-01T21:03:08Z

+                  : (".ctrltext." + std::to_string(colnum) + "." + std::to_string(pagenum));
+          for (uint32_t numbd = 0; numbd < num_entries; ++numbd)
+          {
+            auto label = state->get_label_at(index);


what is the expected ASM format when num_entries > 1? Are we expecting multiple labels? Or one label with mulitple wts_params block, or both? Can we please add a test case with num_entries > 1?

good catch, thought about this found, we can never write a controlcode with num_entries > 1 without tagging them with 2 different labels

larry9523

Besides the num_entries comment, I think copilot has an legitimate comment about reuse patching schema 10 from unknown to pl_addr_64.

While the risk is low, it is still not quite safe if there is any old ELF that has patching schema 10 as a invalid patching when running on new elf parser (xrt). One safe option is to keep 10 as unknown forever and start adding patching schema from 11. Or, we take the low risk to reuse 10, which makes the enum ugly. But to avoid future compatibility issue, we should not use any possible number as unknown. We can choose 0 or -1, for example.

HimanshuChoudhary-Xilinx · 2026-07-03T11:52:20Z

Besides the num_entries comment, I think copilot has an legitimate comment about reuse patching schema 10 from unknown to pl_addr_64.

While the risk is low, it is still not quite safe if there is any old ELF that has patching schema 10 as a invalid patching when running on new elf parser (xrt). One safe option is to keep 10 as unknown forever and start adding patching schema from 11. Or, we take the low risk to reuse 10, which makes the enum ugly. But to avoid future compatibility issue, we should not use any possible number as unknown. We can choose 0 or -1, for example.

we dont generate any any elf with patching schema 10 till now, but we let me make "unknown" as 0xFF so it never conflict with any in future. or just remove "unknown" forever.

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

github-actions

clang-tidy made some suggestions

github-actions · 2026-07-06T12:46:15Z

+  return control_op_apply_offset_pl(
+    pc,
+    /* table_ptr (const) */ *(uint16_t *)(&pc[2]),
+    /* buffer_id (const) */ *(uint16_t *)(&pc[4])


warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

/* buffer_id (const) */ *(uint16_t *)(&pc[4]) ^

Signed-off-by: Ch Vamshi Krishna <40261882+chvamshi-xilinx@users.noreply.github.com>

larry9523 · 2026-07-06T18:43:54Z

Besides the num_entries comment, I think copilot has an legitimate comment about reuse patching schema 10 from unknown to pl_addr_64.
While the risk is low, it is still not quite safe if there is any old ELF that has patching schema 10 as a invalid patching when running on new elf parser (xrt). One safe option is to keep 10 as unknown forever and start adding patching schema from 11. Or, we take the low risk to reuse 10, which makes the enum ugly. But to avoid future compatibility issue, we should not use any possible number as unknown. We can choose 0 or -1, for example.

we dont generate any any elf with patching schema 10 till now, but we let me make "unknown" as 0xFF so it never conflict with any in future. or just remove "unknown" forever.

Let's keep unknown as 10. It is a valid type anyway. And put new type after it. Let's not put _max, or _num at the end to avoid it is mistakenly written to ELF

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

opcode and patching schema for pl ip to transfer wts

1e45efe

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

Copilot AI review requested due to automatic review settings July 1, 2026 12:09

Copilot started reviewing on behalf of HimanshuChoudhary-Xilinx July 1, 2026 12:10 View session

github-actions Bot reviewed Jul 1, 2026

View reviewed changes

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread src/cpp/common/symbol.h Outdated

Comment on lines +24 to +28

control_packet_57 = 7,

address_64 = 8,

control_packet_57_aie4 = 9,

unknown = 10,

pl_ddr_64 = 10,

unknown = 11,

HimanshuChoudhary-Xilinx requested review from larry9523, sonals and xuhz July 1, 2026 13:51

larry9523 reviewed Jul 1, 2026

View reviewed changes

droped num_entries

bbf21e3

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

github-actions Bot reviewed Jul 6, 2026

View reviewed changes

Update ctrlcode.txt with new control codes

2f9928f

Signed-off-by: Ch Vamshi Krishna <40261882+chvamshi-xilinx@users.noreply.github.com>

chvamshi-xilinx approved these changes Jul 6, 2026

View reviewed changes

HimanshuChoudhary-Xilinx added 2 commits July 7, 2026 03:31

fix patching schema

f1c75c5

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

fix

1034c32

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>

larry9523 approved these changes Jul 6, 2026

View reviewed changes

chvamshi-xilinx merged commit 391d3cd into Xilinx:main-ge Jul 7, 2026
6 checks passed

Uh oh!

Conversation

HimanshuChoudhary-Xilinx commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem solved by the commit

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

How problem was solved, alternative solutions (if any) and why they were rejected

Risks (if any) associated the changes in the commit

What has been tested and how, request additional testing if necessary

Documentation impact (if any)

TEST

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

larry9523 Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

HimanshuChoudhary-Xilinx Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

larry9523 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HimanshuChoudhary-Xilinx commented Jul 3, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

larry9523 commented Jul 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HimanshuChoudhary-Xilinx commented Jul 1, 2026 •

edited

Loading

HimanshuChoudhary-Xilinx Jul 3, 2026 •

edited

Loading

larry9523 left a comment •

edited

Loading