Skip to content

add a error if TRACE appears in user generated ctrlcode. TRACE is for internal consumption by CERT. - #360

Open
HimanshuChoudhary-Xilinx wants to merge 3 commits into
Xilinx:main-gefrom
HimanshuChoudhary-Xilinx:trace
Open

add a error if TRACE appears in user generated ctrlcode. TRACE is for internal consumption by CERT.#360
HimanshuChoudhary-Xilinx wants to merge 3 commits into
Xilinx:main-gefrom
HimanshuChoudhary-Xilinx:trace

Conversation

@HimanshuChoudhary-Xilinx

Copy link
Copy Markdown
Collaborator

Problem solved by the commit

Add a error if TRACE appears in user generated ctrlcode. TRACE is for internal consumption by CERT.
Added testcase
Updated Spec

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)

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>
Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>
Copilot AI lite review requested due to automatic review settings August 25, 2026 11:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds assembler rejection for user-authored TRACE opcodes, with a negative test and ISA documentation updates.

Changes:

  • Rejects TRACE during assembly.
  • Adds and registers a negative test.
  • Updates YAML, Markdown, and HTML specifications.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Summary
test/aie2ps-ctrlcode/opcode_checks/trace_in_ctrlcode.asm Adds invalid TRACE input coverage.
test/aie2ps-ctrlcode/opcode_checks/CMakeLists.txt Registers the negative test; should verify the expected diagnostic with PASS_REGULAR_EXPRESSION.
test/aie2ps-ctrlcode/CMakeLists.txt Includes opcode checks.
src/cpp/preprocessor/asm/asm_parser.cpp Rejects user-generated TRACE opcodes.
specification/aie2ps/isa-spec.yaml Documents the restriction; contains thorw typo.
specification/aie2ps/isa-spec.md Documents the restriction; contains thorw typo.
specification/aie2ps/isa-spec.html Updates generated documentation; contains thorw typo.

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

Comment thread specification/aie2ps/isa-spec.html Outdated
Comment thread specification/aie2ps/isa-spec.md Outdated
Comment thread specification/aie2ps/isa-spec.yaml Outdated
add_test(NAME "${TEST_NAME}"
COMMAND aiebu-asm -t aie2ps -o aie_control.elf -c "${CMAKE_CURRENT_SOURCE_DIR}/${ASM_FILE}"
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
set_tests_properties("${TEST_NAME}" PROPERTIES WILL_FAIL TRUE)
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

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

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants