Skip to content

feat: optimize C extensions #1814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Henry Hsieh (henry-hsieh) wants to merge 1 commit into from
Closed

feat: optimize C extensions #1814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions spec/schemas/inst_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"$id": "v0.2",
"$id": "v0.3",
"$defs": {
"fully_resolved_opcodes": {
"type": "object",
Expand Down Expand Up @@ -267,6 +267,15 @@
"default": 0,
"description": "Amount the field should be left shifted before use (e.g., for opcode[5:3], left_shift is 3)"
},
"add": {
"type": "integer",
"default": 0,
"description": "Amount the field should be added before use, e.g., source register is ranged between 8 to 15, add is 8"
},
Comment on lines +270 to +274

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

IMHO, while this is useful in isolation, it presents some challenges by only solving one arithmetic issue, and leaves others (like additional operators and "precedence") unresolved. In #1527, I'm trying to lump all of the operand<->field transformations into new, general encode() and decode() methods. These are IDL, which allows most any transformation, but also complicates their use. Would these methods accommodate your use-case well enough?

@henry-hsieh Henry Hsieh (henry-hsieh) May 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True. encode() and decode() should be enough for this. With these 2 functions, I think left_shift() could be merged into them, too.

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think left_shift() could be merged into them, too.

True.

"implicit": {
"type": "integer",
"description": "Implicitly use an operand with the value"
},
Comment on lines +275 to +278

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we do need something like this. You've applied it to opcode fields, but the variable fields are by definition, variable, and "implicit" by definition is not really variable, so it's not a great fit.

The implicit content seems to be more closely associated with operands (which is the approach in #1527): the input to the instruction.

It is a bit subjective.

Either way, I think it needs to be generalized to at least cover:

  • stack pointer (as you do here)
  • program counter (which can't be represented as an "integer" as is done here)
  • the unmentioned register in register pair usage (this is awkward because it depends on one of the non-implicit operands, so we'll need a way to represent that). I guess this case is sort of implicit and variable.

@henry-hsieh Henry Hsieh (henry-hsieh) May 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm fine with moving it to the fields other than variable. To integrate with new operand field, I have some ideas:

  1. Implicit operand (with new implicit attribute)
$schema: "operand_schema.json#"
kind: operand
name: sp-implicit
data:
  $inherits: operand/xs.yaml#/data
  possible_values: [2]
  implicit: true
  1. Name with multiple of actual registers
  • Introduce name to values mapping
  • The name could be an array for ABI name swapping
  • The values can be an array with all included values
$schema: "operand_schema.json#"
kind: operand
name: xs-pair
data:
  type: reg_pair
  name: reg_pair_xs
  possible_values:
    - $inherits: operand/xs.yaml#/data
      name: ["x0", "zero"]
      values: [0, 1]
    - $inherits: operand/xs.yaml#/data
      name: ["x2", "sp"]
      values: [2, 3]
    - $inherits: operand/xs.yaml#/data
      name: ["x4", "tp"]
      values: [4, 5]
...

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good ideas. Feel free to post them in #1527. (Or, I'll copy them there if you wish.)

@henry-hsieh Henry Hsieh (henry-hsieh) May 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'll copy them to the PR.

"alias": {
"type": "string",
"description": "Alias of the field. Used when a field can represent multiple things, e.g., when a source register is also the destination register"
Expand All @@ -286,7 +295,14 @@
"description": "Specific value(s) that are not permitted for this field."
}
},
"required": ["name", "location"],
"oneOf": [
{
"required": ["name", "location"]
},
{
"required": ["name", "implicit"]
}
],
"additionalProperties": false
}
]
Expand Down
1 change: 1 addition & 0 deletions spec/std/isa/inst/C/c.add.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ encoding:
- name: xd
location: 11-7
not: 0
alias: xs1

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I didn't even know UDB had this feature. Are you adding this just to match the ISA manual? Do you have a use-case for it? Just curious. It seems pretty harmless, so no objections.

@henry-hsieh Henry Hsieh (henry-hsieh) May 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, just to match ISA manual. But I think it also beneficial for someone who wants to use this database for compiler scheduling or RTL decoder generation. For these purposes, you must know these operands are used for both source and destination. I think with new operand system. We could just create a new xsdc operand for this purpose.

$schema: "operand_schema.json#"
kind: operand
name: xsdc
data:
  $inherits: operand/xdc.yaml#/data
  source: true

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agree. I didn't yet model a source/destination register, but I think the capability is already there (as in your example).

access:
s: always
u: always
Expand Down
2 changes: 2 additions & 0 deletions spec/std/isa/inst/C/c.addi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ encoding:
- name: imm
location: 12|6-2
not: 0
sign_extend: true

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These sign_extend fixes should probably go in their own PR, since they are not subjective and we could get them merged much faster.

@henry-hsieh Henry Hsieh (henry-hsieh) May 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll file another PR for this in a few days.

Edit: A few minutes actually 😅

- name: xd
location: 11-7
not: 0
alias: xs1
access:
s: always
u: always
Expand Down
38 changes: 38 additions & 0 deletions spec/std/isa/inst/C/c.addi16sp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,44 @@ encoding:
location: 12|4-3|5|2|6
left_shift: 4
not: 0
sign_extend: true
- name: xd
location: 11-7
Comment on lines +27 to +28

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm guessing this fails to validate properly, as this new "variable" is hardcoded to 2 ("00010") in the "match" string just above. Try running ./do gen:resolved_arch.

An open question, perhaps: where is the syntax defined for c.addi16sp? Is is c.addi16sp sp, imm or c.addi16sp imm? (Why would "sp" need to be an actual operand when it's attached to the mnemonic?

@henry-hsieh Henry Hsieh (henry-hsieh) May 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I forget to change mask "00010" to "-----". I think two approaches are valid:

  1. Keep mask as is, but add implicit operand to this instruction
  2. Change mask to "-----" and add a new operand which only has 1 possible value, i.e., 2.

I don't think c.addi16sp is defined in any official document. I think we use this syntax because official GCC is using it: riscv-collab/riscv-gnu-toolchain#372

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh. Thanks for digging up that (very sad) reference. 😕 Not a choice I would've made.

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a strange case, indeed. Making the field a "variable with only one possible value" seems a little odd. Making "sp" an implicit operand when it is explicitly there also seems odd. This instruction is odd.

I could be swayed either way, but prefer the "variable with one possible value", if you want to update the mask.

@henry-hsieh Henry Hsieh (henry-hsieh) May 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I prefer the "variable with one possible value", too. But I want to use new operand system to do it instead of handcoding 0-31 exclude 2.

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should do both the field and the operand. We do want to properly constrain the field so that simple decode won't match (if the purpose of the decode was to execute rather than disassemble, for example).

not:
[
0,
1,
3,
4,
5,
6,
7,
8,
9,
10,
11,
12,
13,
14,
15,
16,
17,
18,
19,
20,
21,
22,
23,
24,
25,
26,
27,
28,
29,
30,
31,
]
alias: xs1
access:
s: always
u: always
Expand Down
3 changes: 3 additions & 0 deletions spec/std/isa/inst/C/c.addi4spn.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ encoding:
not: 0
- name: xd
location: 4-2
add: 8
- name: xs1
implicit: 2
access:
s: always
u: always
Expand Down
2 changes: 2 additions & 0 deletions spec/std/isa/inst/C/c.addiw.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ encoding:
variables:
- name: imm
location: 12|6-2
sign_extend: true
- name: xd
location: 11-7
not: 0
alias: xs1
access:
s: always
u: always
Expand Down
3 changes: 3 additions & 0 deletions spec/std/isa/inst/C/c.addw.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ encoding:
variables:
- name: xs2
location: 4-2
add: 8
- name: xd
location: 9-7
add: 8
alias: xs1
access:
s: always
u: always
Expand Down
3 changes: 3 additions & 0 deletions spec/std/isa/inst/C/c.and.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ encoding:
variables:
- name: xs2
location: 4-2
add: 8
- name: xd
location: 9-7
add: 8
alias: xs1
access:
s: always
u: always
Expand Down
3 changes: 3 additions & 0 deletions spec/std/isa/inst/C/c.andi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ encoding:
variables:
- name: imm
location: 12|6-2
sign_extend: true
- name: xd
location: 9-7
add: 8
alias: xs1
access:
s: always
u: always
Expand Down
3 changes: 3 additions & 0 deletions spec/std/isa/inst/C/c.beqz.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ encoding:
sign_extend: true
- name: xs1
location: 9-7
add: 8
- name: xs2
implicit: 0
Comment on lines +27 to +28

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

c.beqz does not have an xs2. Are you trying to line up for a theoretical expansion to the base instruction, beq? That seems unnecessary here to me.

There are a number of changes like this.

@henry-hsieh Henry Hsieh (henry-hsieh) May 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I just want to align the theoretical expansion to the base instruction. But it's totally fine not to do this.

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's not. I think it would be confusing. There is already enough information to get the right values for all of the operands in the base instruction.

access:
s: always
u: always
Expand Down
3 changes: 3 additions & 0 deletions spec/std/isa/inst/C/c.bnez.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ encoding:
sign_extend: true
- name: xs1
location: 9-7
add: 8
- name: xs2
implicit: 0
access:
s: always
u: always
Expand Down
2 changes: 2 additions & 0 deletions spec/std/isa/inst/C/c.j.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ encoding:
location: 12|8|10-9|6|7|2|11|5-3
left_shift: 1
sign_extend: true
- name: xd
implicit: 0
access:
s: always
u: always
Expand Down
2 changes: 2 additions & 0 deletions spec/std/isa/inst/C/c.jal.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ encoding:
location: 12|8|10-9|6|7|2|11|5-3
left_shift: 1
sign_extend: true
- name: xd
implicit: 1
access:
s: always
u: always
Expand Down
2 changes: 2 additions & 0 deletions spec/std/isa/inst/C/c.jalr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ encoding:
- name: xs1
location: 11-7
not: 0
- name: xd
implicit: 1
access:
s: always
u: always
Expand Down
2 changes: 2 additions & 0 deletions spec/std/isa/inst/C/c.jr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ encoding:
- name: xs1
location: 11-7
not: 0
- name: xd
implicit: 0
access:
s: always
u: always
Expand Down
4 changes: 4 additions & 0 deletions spec/std/isa/inst/C/c.ld.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@ encoding:
- name: xd
location: 4-2
not: [1, 3, 5, 7]
add: 8
- name: xs1
location: 9-7
add: 8
RV64:
match: 011-----------00
variables:
Expand All @@ -42,8 +44,10 @@ encoding:
left_shift: 3
- name: xd
location: 4-2
add: 8
- name: xs1
location: 9-7
add: 8
access:
s: always
u: always
Expand Down
6 changes: 5 additions & 1 deletion spec/std/isa/inst/C/c.ldsp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ encoding:
left_shift: 3
- name: xd
location: 11-7
not: [0, 1, 3, 5, 7]
not: [0, 1, 3, 5, 7, 9, 11, 13, 15, 17, 19, 21, 23, 25, 27, 29, 31]
Comment thread
ThinkOpenly marked this conversation as resolved.
- name: xs1
implicit: 2
RV64:
match: 011-----------10
variables:
Expand All @@ -43,6 +45,8 @@ encoding:
left_shift: 3
- name: xd
location: 11-7
- name: xs1
implicit: 2
access:
s: always
u: always
Expand Down
3 changes: 3 additions & 0 deletions spec/std/isa/inst/C/c.li.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ encoding:
variables:
- name: imm
location: 12|6-2
sign_extend: true
- name: xd
location: 11-7
not: 0
- name: xs1
implicit: 0
access:
s: always
u: always
Expand Down
1 change: 1 addition & 0 deletions spec/std/isa/inst/C/c.lui.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ encoding:
- name: imm
location: 12|6-2
left_shift: 12
sign_extend: true
- name: xd
location: 11-7
not: [0, 2]
Expand Down
2 changes: 2 additions & 0 deletions spec/std/isa/inst/C/c.lw.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ encoding:
left_shift: 2
- name: xd
location: 4-2
add: 8
- name: xs1
location: 9-7
add: 8
access:
s: always
u: always
Expand Down
2 changes: 2 additions & 0 deletions spec/std/isa/inst/C/c.lwsp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ encoding:
- name: xd
location: 11-7
not: 0
- name: xs1
implicit: 2
access:
s: always
u: always
Expand Down
4 changes: 3 additions & 1 deletion spec/std/isa/inst/C/c.mv.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ name: c.mv
long_name: Move Register
description: |
C.MV (move register) performs copy of the data in register xs2 to register xd
C.MV expands to addi xd, x0, xs2.
C.MV expands to add xd, x0, xs2.

@ThinkOpenly Paul Clarke (ThinkOpenly) May 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Another bug fix for a separate PR.

definedBy:
extension:
name: Zca
Expand All @@ -20,6 +20,8 @@ encoding:
- name: xd
location: 11-7
not: 0
- name: xs1
implicit: 0
- name: xs2
location: 6-2
not: 0
Expand Down
3 changes: 3 additions & 0 deletions spec/std/isa/inst/C/c.or.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ encoding:
variables:
- name: xs2
location: 4-2
add: 8
- name: xd
location: 9-7
add: 8
alias: xs1
access:
s: always
u: always
Expand Down
4 changes: 4 additions & 0 deletions spec/std/isa/inst/C/c.sd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ encoding:
left_shift: 3
- name: xs2
location: 4-2
add: 8
not: [1, 3, 5, 7]
- name: xs1
location: 9-7
add: 8
RV64:
match: 111-----------00
variables:
Expand All @@ -41,8 +43,10 @@ encoding:
left_shift: 3
- name: xs2
location: 4-2
add: 8
- name: xs1
location: 9-7
add: 8
access:
s: always
u: always
Expand Down
6 changes: 5 additions & 1 deletion spec/std/isa/inst/C/c.sdsp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ encoding:
variables:
- name: xs2
location: 6-2
not: [1, 3, 5, 7]
not: [1, 3, 5, 7, 9, 11, 13, 15, 17, 19, 21, 23, 25, 27, 29, 31]
Comment thread
ThinkOpenly marked this conversation as resolved.
- name: imm
location: 9-7|12-10
left_shift: 3
- name: xs1
implicit: 2
RV64:
match: 111-----------10
variables:
Expand All @@ -39,6 +41,8 @@ encoding:
- name: imm
location: 9-7|12-10
left_shift: 3
- name: xs1
implicit: 2
access:
s: always
u: always
Expand Down
2 changes: 2 additions & 0 deletions spec/std/isa/inst/C/c.slli.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ encoding:
not: 0
- name: xd
location: 11-7
alias: xs1
RV64:
match: 000-----------10
variables:
Expand All @@ -31,6 +32,7 @@ encoding:
not: 0
- name: xd
location: 11-7
alias: xs1
access:
s: always
u: always
Expand Down
Loading