[go] cmd/internal/obj/riscv: add support for compressed instructions
Joel Sing (Gerrit)
Attention is currently required from: Cherry Mui, M Zhuo, Mark Ryan.
Joel Sing would like Mark Ryan, M Zhuo and Cherry Mui to review this change.
cmd/internal/obj/riscv: add support for compressed instructions
Implement validation and encoding for Compressed Immediate (CI)
instructions, adding support for the compressed NOP instruction
(CNOP).
Reduce the PC quantum to two bytes, reduce the minimum instruction
length to two bytes and use CNOP in runtime.goexit, where the NOPs
need to match the length of the PC quantum.
Note that this moves the baseline for the riscv64 port to rv64gc.
Change-Id: I4329a8fbfcb4de636aadaeadabb826bc22698640
---
M src/cmd/asm/internal/asm/testdata/riscv64.s
M src/cmd/internal/obj/riscv/obj.go
M src/cmd/internal/sys/arch.go
M src/internal/goarch/goarch_riscv64.go
M src/runtime/asm_riscv64.s
5 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/src/cmd/asm/internal/asm/testdata/riscv64.s b/src/cmd/asm/internal/asm/testdata/riscv64.s
index 9899ec9..e80aad9 100644
--- a/src/cmd/asm/internal/asm/testdata/riscv64.s
+++ b/src/cmd/asm/internal/asm/testdata/riscv64.s
@@ -294,6 +294,9 @@
// 12.6: Double-Precision Floating-Point Classify Instruction
FCLASSD F0, X5 // d31200e2
+ // 16.5: Compressed - Integer Computational Instructions
+ CNOP // 0100
+
// Privileged ISA
// 3.2.1: Environment Call and Breakpoint
diff --git a/src/cmd/internal/obj/riscv/obj.go b/src/cmd/internal/obj/riscv/obj.go
index 776c3a8..1fc27ca 100644
--- a/src/cmd/internal/obj/riscv/obj.go
+++ b/src/cmd/internal/obj/riscv/obj.go
@@ -1062,6 +1062,15 @@
}
}
+func validateCI(ctxt *obj.Link, ins *instruction) {
+ if ins.as == ACNOP && (ins.rd != REG_X0 || ins.rs1 != REG_X0 || ins.imm != 0) {
+ ctxt.Diag("%v: must use X0 register and have a zero immediate", ins.as)
+ }
+ wantIntReg(ctxt, ins.as, "rd", ins.rd)
+ wantIntReg(ctxt, ins.as, "rs1", ins.rs1)
+ wantNoneReg(ctxt, ins.as, "rs2", ins.rs2)
+}
+
func validateRIII(ctxt *obj.Link, ins *instruction) {
wantIntReg(ctxt, ins.as, "rd", ins.rd)
wantIntReg(ctxt, ins.as, "rs1", ins.rs1)
@@ -1187,6 +1196,16 @@
return ((imm >> bit) & 1) << pos
}
+// encodeCI encodes a compressed immediate (CI-type) instruction.
+func encodeCI(ins *instruction) uint32 {
+ enc := encode(ins.as)
+ if enc == nil {
+ panic("encodeCI: could not encode instruction")
+ }
+ imm := uint32(ins.imm)
+ return enc.funct3<<12 | ((imm&0x20)>>5)<<12 | regI(ins.rd)<<7 | (imm&0x1f)<<2 | enc.opcode
+}
+
// encodeR encodes an R-type RISC-V instruction.
func encodeR(as obj.As, rs1, rs2, rd, funct3, funct7 uint32) uint32 {
enc := encode(as)
@@ -1430,7 +1449,7 @@
type encoding struct {
encode func(*instruction) uint32 // encode returns the machine code for an instruction
validate func(*obj.Link, *instruction) // validate validates an instruction
- length int // length of encoded instruction; 0 for pseudo-ops, 4 otherwise
+ length int // length of encoded instruction; 0 for pseudo-ops, 2 for compressed instructions, 4 otherwise
}
var (
@@ -1463,6 +1482,9 @@
uEncoding = encoding{encode: encodeU, validate: validateU, length: 4}
jEncoding = encoding{encode: encodeJ, validate: validateJ, length: 4}
+ // Compressed encodings.
+ ciEncoding = encoding{encode: encodeCI, validate: validateCI, length: 2}
+
// rawEncoding encodes a raw instruction byte sequence.
rawEncoding = encoding{encode: encodeRawIns, validate: validateRaw, length: 4}
@@ -1672,6 +1694,9 @@
// 12.7: Double-Precision Floating-Point Classify Instruction
AFCLASSD & obj.AMask: rFIEncoding,
+ // 16.5: Compressed - Integer Computational Instructions
+ ACNOP & obj.AMask: ciEncoding,
+
// Privileged ISA
// 3.2.1: Environment Call and Breakpoint
@@ -2319,6 +2344,9 @@
if ins.imm < 0 || ins.imm > 31 {
p.Ctxt.Diag("%v: shift amount out of range 0 to 31", p)
}
+
+ case ACNOP:
+ ins.rd, ins.rs1 = REG_ZERO, REG_ZERO
}
return inss
}
diff --git a/src/cmd/internal/sys/arch.go b/src/cmd/internal/sys/arch.go
index 2e35284..77ff223 100644
--- a/src/cmd/internal/sys/arch.go
+++ b/src/cmd/internal/sys/arch.go
@@ -234,7 +234,7 @@
ByteOrder: binary.LittleEndian,
PtrSize: 8,
RegSize: 8,
- MinLC: 4,
+ MinLC: 2,
Alignment: 8, // riscv unaligned loads work, but are really slow (trap + simulated by OS)
CanMergeLoads: false,
HasLR: true,
diff --git a/src/internal/goarch/goarch_riscv64.go b/src/internal/goarch/goarch_riscv64.go
index 3b6da1e..468f9a6 100644
--- a/src/internal/goarch/goarch_riscv64.go
+++ b/src/internal/goarch/goarch_riscv64.go
@@ -7,7 +7,7 @@
const (
_ArchFamily = RISCV64
_DefaultPhysPageSize = 4096
- _PCQuantum = 4
+ _PCQuantum = 2
_MinFrameSize = 8
_StackAlign = PtrSize
)
diff --git a/src/runtime/asm_riscv64.s b/src/runtime/asm_riscv64.s
index eb53cbb..7890058 100644
--- a/src/runtime/asm_riscv64.s
+++ b/src/runtime/asm_riscv64.s
@@ -509,10 +509,10 @@
// The top-most function running on a goroutine
// returns to goexit+PCQuantum.
TEXT runtime·goexit(SB),NOSPLIT|NOFRAME|TOPFRAME,$0-0
- MOV ZERO, ZERO // NOP
+ CNOP // 2 byte compressed NOP
JMP runtime·goexit1(SB) // does not return
// traceback from goexit1 must hit code range of goexit
- MOV ZERO, ZERO // NOP
+ CNOP // 2 byte compressed NOP
// func cgocallback(fn, frame unsafe.Pointer, ctxt uintptr)
// See cgocall.go for more details.
To view, visit change 523477. To unsubscribe, or for help writing mail filters, visit settings.
Joel Sing (Gerrit)
Joel Sing uploaded new patchset![]( "Open in Gerrit")
- Cherry Mui
- Joel Sing
- Mark Ryan
- Meng Zhuo
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing uploaded new patchset![]( "Open in Gerrit")
Related details
- Cherry Mui
- Joel Sing
- Mark Ryan
- Meng Zhuo
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing uploaded new patchset![]( "Open in Gerrit")
Related details
- Cherry Mui
- Joel Sing
- Mark Ryan
- Meng Zhuo
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing uploaded new patchset![]( "Open in Gerrit")
Related details
- Cherry Mui
- Joel Sing
- Mark Ryan
- Meng Zhuo
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing removed a vote from this change![]( "Open in Gerrit")
- Cherry Mui
- Joel Sing
- Mark Ryan
- Meng Zhuo
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing voted and added 1 comment![]( "Open in Gerrit")
Votes added by Joel Sing
| Commit-Queue | +1 |
1 comment
Joel SingThis would require compressed instruction support in the riscv port, so we need an accepted proposal for that.
Proposal has been accepted, removing hold.
Related details
- Cherry Mui
- Mark Ryan
- Meng Zhuo
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Cherry Mui
- Mark Ryan
- Meng Zhuo
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cherry Mui (Gerrit)
Cherry Mui added 2 comments![]( "Open in Gerrit")
As mentioned on the issue, we probably want to have a command-line debugging flag to switch on/off compressed instructions.
CNOP // 2 byte compressed NOPCould the assembler just emit CNOP for `MOV ZERO, ZERO`? This way, we can have it controlled by a command-line flag in the assembler, and the runtime code doesn't need to change.
Related details
- Joel Sing
- Mark Ryan
- Meng Zhuo
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing added 1 comment![]( "Open in Gerrit")
CNOP // 2 byte compressed NOPCould the assembler just emit CNOP for `MOV ZERO, ZERO`? This way, we can have it controlled by a command-line flag in the assembler, and the runtime code doesn't need to change.
I don't believe so - I'll double check (it's been 2+ years since I originally did this work), but as far as I recall the size of the NOPs here have to be exactly the same as the PC quantum. If this is correct we cannot flip between a CNOP (2 bytes) and regular ADDI based noop (4 bytes) without also changing the PC quantum... but let me confirm.
Related details
- Cherry Mui
- Mark Ryan
- Meng Zhuo
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing voted and added 2 comments![]( "Open in Gerrit")
Votes added by Joel Sing
| Commit-Queue | +1 |
As mentioned on the issue, we probably want to have a command-line debugging flag to switch on/off compressed instructions.
This certainly makes sense to disable the compression pass that will be added to the assembler to emit compressed instructions that replace regular instructions. I think CNOPs have to always be available though.
CNOP // 2 byte compressed NOPJoel SingCould the assembler just emit CNOP for `MOV ZERO, ZERO`? This way, we can have it controlled by a command-line flag in the assembler, and the runtime code doesn't need to change.
I don't believe so - I'll double check (it's been 2+ years since I originally did this work), but as far as I recall the size of the NOPs here have to be exactly the same as the PC quantum. If this is correct we cannot flip between a CNOP (2 bytes) and regular ADDI based noop (4 bytes) without also changing the PC quantum... but let me confirm.
```
$ ./all.bash
Building Go cmd/dist using /usr/local/go. (go1.24.6 openbsd/riscv64)
Building Go toolchain1 using /usr/local/go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
SIGILL: illegal instruction
PC=0x8a922 m=0 sigcode=4
instruction bytes: 0x0 0x0 0x6f 0x20 0x40 0x6e 0x13 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x73 0x0
```
As expected, we end up in the middle of the 4 byte instruction which is then SIGILL:
```
000000000008a920 <runtime.goexit.abi0>:
8a920: 00000013 nop
8a924: 6e40206f j 0x8d008 <runtime.goexit1.abi0>
8a928: 00000013 nop
8a92c: 0000 <unknown>
8a92e: 0000 <unknown>
```
Related details
- Cherry Mui
- Mark Ryan
- Meng Zhuo
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
- Cherry Mui
- Mark Ryan
- Meng Zhuo
- Russ Cox
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Cherry Mui
- Mark Ryan
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing removed a vote from this change![]( "Open in Gerrit")
Removed Commit-Queue+1 by Joel Sing <jo...@sing.id.au>
Related details
- Cherry Mui
- Mark Ryan
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing voted and added 2 comments![]( "Open in Gerrit")
Votes added by Joel Sing
| Commit-Queue | +1 |
Joel SingAs mentioned on the issue, we probably want to have a command-line debugging flag to switch on/off compressed instructions.
This certainly makes sense to disable the compression pass that will be added to the assembler to emit compressed instructions that replace regular instructions. I think CNOPs have to always be available though.
This is included in the next change (as `-d=compressinstructions=0`)
CNOP // 2 byte compressed NOPJoel SingCould the assembler just emit CNOP for `MOV ZERO, ZERO`? This way, we can have it controlled by a command-line flag in the assembler, and the runtime code doesn't need to change.
Joel SingI don't believe so - I'll double check (it's been 2+ years since I originally did this work), but as far as I recall the size of the NOPs here have to be exactly the same as the PC quantum. If this is correct we cannot flip between a CNOP (2 bytes) and regular ADDI based noop (4 bytes) without also changing the PC quantum... but let me confirm.
```
$ ./all.bash
Building Go cmd/dist using /usr/local/go. (go1.24.6 openbsd/riscv64)
Building Go toolchain1 using /usr/local/go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
SIGILL: illegal instruction
PC=0x8a922 m=0 sigcode=4
instruction bytes: 0x0 0x0 0x6f 0x20 0x40 0x6e 0x13 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x73 0x0
```As expected, we end up in the middle of the 4 byte instruction which is then SIGILL:
```
000000000008a920 <runtime.goexit.abi0>:
8a920: 00000013 nop
8a924: 6e40206f j 0x8d008 <runtime.goexit1.abi0>
8a928: 00000013 nop
8a92c: 0000 <unknown>
8a92e: 0000 <unknown>
```
Resolving since there is no obvious way to work around this easily, and the effort is not justified.
Related details
- Cherry Mui
- Joel Sing
- Mark Ryan
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing removed a vote from this change![]( "Open in Gerrit")
Removed LUCI-TryBot-Result-1 by Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Related details
- Cherry Mui
- Joel Sing
- Mark Ryan
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing voted Commit-Queue+1![]( "Open in Gerrit")
- Cherry Mui
- Mark Ryan
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cherry Mui (Gerrit)
Cherry Mui added 1 comment![]( "Open in Gerrit")
CNOP // 2 byte compressed NOPAs discussed in the issue, we want to have a flag in the compiler/assembler to control whether to use compressed instructions. If this is the only use in the runtime assembly, probably just leave them.
Related details
- Joel Sing
- Mark Ryan
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing added 1 comment![]( "Open in Gerrit")
CNOP // 2 byte compressed NOPAs discussed in the issue, we want to have a flag in the compiler/assembler to control whether to use compressed instructions. If this is the only use in the runtime assembly, probably just leave them.
The previous comment threads explain this in detail - please see:
https://go-review.googlesource.com/c/go/+/523477/comment/4159da8c_7a60a1df
https://go-review.googlesource.com/c/go/+/523477/comments/765358be_38dd1e42
In summary, I do not believe there is an easy way to avoid this usage in the runtime (or at least it would require significant effort) and the debug flag is included in the next CL (https://go-review.googlesource.com/c/go/+/523478/10).
Related details
- Cherry Mui
- Mark Ryan
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cherry Mui (Gerrit)
Cherry Mui voted and added 1 comment![]( "Open in Gerrit")
Votes added by Cherry Mui
| Code-Review | +2 |
1 comment
CNOP // 2 byte compressed NOPJoel SingAs discussed in the issue, we want to have a flag in the compiler/assembler to control whether to use compressed instructions. If this is the only use in the runtime assembly, probably just leave them.
The previous comment threads explain this in detail - please see:
https://go-review.googlesource.com/c/go/+/523477/comment/4159da8c_7a60a1dfhttps://go-review.googlesource.com/c/go/+/523477/comments/765358be_38dd1e42In summary, I do not believe there is an easy way to avoid this usage in the runtime (or at least it would require significant effort) and the debug flag is included in the next CL (https://go-review.googlesource.com/c/go/+/523478/10).
Sorry about commenting on it again. And thanks for the explanation. I think I see what the problem is without this change.
Suppose one wants to run Go on a chip without compressed instruction support (not saying we should support this case, just suppose). The CNOP instructions here are actually not executed, as we manually construct a return address to point to goexit+PCQuantum. So the CNOP won't cause an illegal instruction. However, as PCQuantum is 2, it will be an unaligned address of a non-compressed instruction, so the machine will still fault, right?
If we want to support this case, one thing we could do is in gostartcall ( https://cs.opensource.google/go/go/+/master:src/runtime/sys_riscv64.go;l=11 ), change buf.lr to be goexit+4 (on entry buf.pc is goexit+PCQuantum as prepared by newproc1). Not sure it is worth the effort.
Related details
- Joel Sing
- Mark Ryan
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Ryan (Gerrit)
Mark Ryan voted and added 1 comment![]( "Open in Gerrit")
- Joel Sing
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing uploaded new patchset![]( "Open in Gerrit")
- Code-Review: +2 by Mark Ryan, +2 by Cherry Mui
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
- Cherry Mui
- Joel Sing
- Mark Ryan
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing voted Commit-Queue+1![]( "Open in Gerrit")
- Cherry Mui
- Mark Ryan
- Meng Zhuo
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |