feat: add model-instruction-from-spec skill#1757
feat: add model-instruction-from-spec skill#1757adingank-qualcomm wants to merge 12 commits intoriscv:mainfrom
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a Claude skill that generates IDL operation() bodies for RISC-V instructions from their YAML spec files, writing to the spec if the operation() key is empty or to /tmp/<name>.yaml if already populated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
adingank-qualcomm
requested review from
ThinkOpenly and
dhower-qc
as code owners
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1757 +/- ##
=======================================
Coverage 71.94% 71.94%
=======================================
Files 54 54
Lines 27976 27976
Branches 6183 6183
=======================================
Hits 20128 20128
Misses 7848 7848
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| Single-letter tokens (`R`, `I`, `S`, `B`, `U`, `J`) are format names, not instructions — exclude them. | ||
|
|
||
| Two-letter tokens that are register names (`x0`–`x31`, `rd`, `rs1`, `rs2`) — exclude them. |
There was a problem hiding this comment.
These aren't all "two-letter tokens". :-)
There was a problem hiding this comment.
Removed the mention of length of tokens.
|
|
||
| Any token introduced by "pseudoinstruction" (with or without "assembler") must be excluded, even if it appears elsewhere in the subsection outside a pseudoinstruction context. Collect all pseudoinstruction names first, then exclude them from the final list. | ||
|
|
||
| **Definitive exclusion list — never treat these as instructions:** |
There was a problem hiding this comment.
"Definitive" seems too strong here and could be interpreted to override the preceding paragraph. Maybe "The following tokens should never be considered instruction names..."
There was a problem hiding this comment.
Fed this comment directly to the agent (or "The Agent" ;-) and got it done!
Perhaps an overkill, but it saved some context switching for me to do these
changes manually!
| Copyright (c) 2026 Qualcomm Technologies, Inc. and/or its subsidiaries. | ||
| SPDX-License-Identifier: BSD-3-Clause | ||
|
|
||
| Given a RISC-V instruction name, locate its YAML spec file under `spec/std/isa/inst/`, read the instruction's description, encoding, and any existing `sail()` implementation, then generate a correct IDL `operation()` body using the IDL language defined by the Treetop grammar in `tools/ruby-gems/idlc/lib/idlc/idl.treetop`. |
There was a problem hiding this comment.
sail() is being removed from UDB.
There was a problem hiding this comment.
Removed ref.
| - **`definedBy`**: The extension(s) that define this instruction. | ||
| - **`access`**: Privilege levels at which the instruction is accessible. | ||
| - **`operation()`**: The current value — check whether it is empty (just `|` with no following content before the next key) or already populated. | ||
| - **`sail()`**: If present, the Sail formal semantics — use this as a reference for the intended behavior. |
There was a problem hiding this comment.
sail() is being removed from UDB.
There was a problem hiding this comment.
Done! Removed references to Sail from other parts also.
| - Custom struct/enum types (e.g., `VectorState`, `VmaOrderType`) | ||
|
|
||
| **Register access:** | ||
| - `X[reg_name]` — read/write integer register (e.g., `X[xs1]`, `X[xd]`) |
There was a problem hiding this comment.
should you add float register file here?
There was a problem hiding this comment.
Added; it will show up in the next commit which will address all of your comments from yesterday.
| # IDL code here | ||
| XReg src = X[xs1]; | ||
| X[xd] = src + 1; | ||
|
|
There was a problem hiding this comment.
any special reason for the blank line here?
There was a problem hiding this comment.
oh, you mention it right below. heh. Not sure it's needed but NBD.
There was a problem hiding this comment.
The agent included the blank line for a convention which improves readability
(or eye strain, I think ;-) so I'm leaning towards leaving it in.
- model-instruction-from-spec: remove Sail references (being removed from UDB YAML files), add floating-point register access syntax - extract-instructions-from-subsection: soften exclusion list heading, drop "Two-letter" qualifier from register name exclusion rule Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@ThinkOpenly I had another chat with "The Agent" to address all of your comments from yesterday. |
2 participants
Summary
.claude/skills/model-instruction-from-spec/SKILL.mdthat generates IDLoperation()bodies for RISC-V instructions from their YAML spec filesspec/std/isa/inst/, reads the description, encoding, andsail()reference, then generates syntactically valid IDL per the grammar intools/ruby-gems/idlc/lib/idlc/idl.treetopoperation()is empty, or to/tmp/<name>.yamlif already populatedTest plan
/model-instruction-from-spec <instruction>on an instruction with an emptyoperation()and verify IDL is written into the spec YAML/model-instruction-from-spec <instruction>on an instruction with a populatedoperation()and verify output goes to/tmp/<name>.yaml🤖 Generated with Claude Code