Skip to content

[Comgr] Port isa_name_parsing_test to LIT and add SPIR-V identifier coverage#2710

Open
theSK2005 wants to merge 3 commits into
amd-stagingfrom
skushwah/lit_isa_name_parsing
Open

[Comgr] Port isa_name_parsing_test to LIT and add SPIR-V identifier coverage#2710
theSK2005 wants to merge 3 commits into
amd-stagingfrom
skushwah/lit_isa_name_parsing

Conversation

@theSK2005
Copy link
Copy Markdown

Ported isa_name_parsing_test from CTest to LIT. Adjusted CMakeList.txt files appropriately.

…overage

Ported isa_name_parsing_test from CTest to LIT. Adjusted CMakeList.txt files appropriately.
@theSK2005 theSK2005 force-pushed the skushwah/lit_isa_name_parsing branch from 7b56ef5 to 4988e3f Compare May 28, 2026 22:28
Comment thread amd/comgr/src/comgr.cpp
Comment thread amd/comgr/test-lit/parse-isa-name.c Outdated
Comment thread amd/comgr/test-lit/parse-isa-name.c
Comment thread amd/comgr/test-lit/parse-isa-name.c
Comment thread amd/comgr/test-lit/comgr-sources/parse-isa-name.c
@theSK2005 theSK2005 requested a review from lamb-j May 29, 2026 16:44
const char *reason = "";

if (argc != 3) {
fprintf(stderr, "Usage: parse-isa-name <isa-name> <expected-status>\n");
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Should I replace this fprintf and exit with a fail?

lamb-j added a commit that referenced this pull request May 29, 2026
The short-circuit in `amd_comgr_action_info_set_isa_name` checked for
`spir64-...` but the canonical HIP SPIR-V triple is `spirv64-...` (LLVM
`Triple::spirv64`) — matching `parseTargetIdentifier` (SWDEV-533122) and
the strings CLR passes in. Introduced as a typo in 967115f.

Dead code on real inputs: nothing in CLR or the LLVM amd subtree passes
`spir64-amd-amdhsa-...amdgcnspirv`. Fix aligns the two Comgr call sites
with clang and CLR.

Discovered while reviewing #2710.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comgr Related to Code Object Manager

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants