[go] cpu/internal: provide runtime detection of RISC-V extensions on Linux
Mark Ryan (Gerrit)
Mark Ryan has uploaded this change for review.
cpu/internal: provide runtime detection of RISC-V extensions on Linux
Add a RISCV64 variable to cpu/internal that indicates both the presence of RISC-V
extensions and performance information about the underlying RISC-V cores.
The variable is only populated with non false values on Linux. The detection code
first attempts to use the riscv_hwprobe syscall introduced in Linux 6.4, falling
back to HWCAP if riscv_hwprobe is not supported. The patch can detect the C, V,
Zba, Zbb and Zbs extensions. V, Zba, Zbb and Zbs can only be detected on a 6.5
kernel or later (without backports).
Updates #61416
Change-Id: I2d8289345c885b699afff441d417cae38f6bdc54
---
M src/internal/cpu/cpu.go
M src/internal/cpu/cpu_riscv64.go
A src/internal/cpu/cpu_riscv64_linux.go
A src/internal/cpu/cpu_riscv64_other.go
M src/runtime/os_linux_noauxv.go
M src/runtime/os_linux_riscv64.go
6 files changed, 164 insertions(+), 1 deletion(-)
diff --git a/src/internal/cpu/cpu.go b/src/internal/cpu/cpu.go
index 1352810..5299f45 100644
--- a/src/internal/cpu/cpu.go
+++ b/src/internal/cpu/cpu.go
@@ -117,6 +117,19 @@
_ CacheLinePad
}
+// The booleans in RISCV64, with the exception of HasMisalignedFast, indicate the presence
+// of RISC-V extensions.
+var RISCV64 struct {
+ _ CacheLinePad
+ HasMisalignedFast bool // Fast misaligned accesses
+ HasC bool // The C extension
+ HasV bool // Vector extension
+ HasZBA bool // The Zba address generation extension
+ HasZBB bool // The Zbb extension
+ HasZBS bool // The Zbs extension
+ _ CacheLinePad
+}
+
// Initialize examines the processor and sets the relevant variables above.
// This is called by the runtime package early in program initialization,
// before normal init functions are run. env is set by runtime if the OS supports
diff --git a/src/internal/cpu/cpu_riscv64.go b/src/internal/cpu/cpu_riscv64.go
index 54b8c33..ff60a89 100644
--- a/src/internal/cpu/cpu_riscv64.go
+++ b/src/internal/cpu/cpu_riscv64.go
@@ -6,5 +6,24 @@
const CacheLinePadSize = 32
+// RISC-V doesn't have a 'cpuid' equivalent. On Linux we rely on the riscv_hwprobe syscall falling
+// back to HWCAP where riscv_hwprobe is not available.
+
+// This value initialized by archauxv() and should not be changed after it is initialized.
+var HWCap uint
+
func doinit() {
+ options = []option{
+ {Name: "misalignedFast", Feature: &RISCV64.HasMisalignedFast},
+ {Name: "c", Feature: &RISCV64.HasC},
+ {Name: "v", Feature: &RISCV64.HasV},
+ {Name: "zba", Feature: &RISCV64.HasZBA},
+ {Name: "zbb", Feature: &RISCV64.HasZBB},
+ {Name: "zbs", Feature: &RISCV64.HasZBS},
+ }
+ osInit()
+}
+
+func isSet(hwc uint, value uint) bool {
+ return hwc&value != 0
}
diff --git a/src/internal/cpu/cpu_riscv64_linux.go b/src/internal/cpu/cpu_riscv64_linux.go
new file mode 100644
index 0000000..85982cb
--- /dev/null
+++ b/src/internal/cpu/cpu_riscv64_linux.go
@@ -0,0 +1,87 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build riscv64 && linux
+
+package cpu
+
+// RISC-V extension discovery code for Linux. The approach here is to first try the riscv_hwprobe
+// syscall falling back to hwcap to check for the C and V extensions if riscv_hwprobe is not available.
+// Please see https://docs.kernel.org/riscv/hwprobe.html for more information.
+
+const (
+ // CPU features
+ hwcap_RISCV_ISA_C = 1 << ('C' - 'A')
+ hwcap_RISCV_ISA_V = 1 << ('V' - 'A')
+)
+
+// riscv_hwprobe keys and values
+const (
+ RISCV_HWPROBE_KEY_MVENDORID = 0x0
+ RISCV_HWPROBE_KEY_MARCHID = 0x1
+ RISCV_HWPROBE_KEY_MIMPID = 0x2
+ RISCV_HWPROBE_KEY_BASE_BEHAVIOR = 0x3
+ RISCV_HWPROBE_BASE_BEHAVIOR_IMA = 0x1
+ RISCV_HWPROBE_KEY_IMA_EXT_0 = 0x4
+ RISCV_HWPROBE_IMA_FD = 0x1
+ RISCV_HWPROBE_IMA_C = 0x2
+ RISCV_HWPROBE_IMA_V = 0x4
+ RISCV_HWPROBE_EXT_ZBA = 0x8
+ RISCV_HWPROBE_EXT_ZBB = 0x10
+ RISCV_HWPROBE_EXT_ZBS = 0x20
+ RISCV_HWPROBE_KEY_CPUPERF_0 = 0x5
+ RISCV_HWPROBE_MISALIGNED_UNKNOWN = 0x0
+ RISCV_HWPROBE_MISALIGNED_EMULATED = 0x1
+ RISCV_HWPROBE_MISALIGNED_SLOW = 0x2
+ RISCV_HWPROBE_MISALIGNED_FAST = 0x3
+ RISCV_HWPROBE_MISALIGNED_UNSUPPORTED = 0x4
+ RISCV_HWPROBE_MISALIGNED_MASK = 0x7
+)
+
+type hwprobePair = struct {
+ key int64
+ value uint64
+}
+
+func RISCVHWProbe(pairs []hwprobePair, flags uint) bool
+
+func osInit() {
+
+ // A slice of key/value pair structures is passed to the RISCVHWProbe syscall. The key
+ // field should be initialised with one of the key constants defined above, e.g.,
+ // RISCV_HWPROBE_KEY_IMA_EXT_0. The syscall will set the value field to the appropriate value.
+ // If the kernel does not recognize a key it will set the key field to -1 and the value field to 0.
+
+ pairs := []hwprobePair{
+ {RISCV_HWPROBE_KEY_IMA_EXT_0, 0},
+ {RISCV_HWPROBE_KEY_CPUPERF_0, 0},
+ }
+
+ // This call only indicates that extensions are supported if they are implemented on all cores.
+ if RISCVHWProbe(pairs, 0) {
+ if pairs[0].key != -1 {
+ v := uint(pairs[0].value)
+ RISCV64.HasC = isSet(v, RISCV_HWPROBE_IMA_C)
+ RISCV64.HasV = isSet(v, RISCV_HWPROBE_IMA_V)
+ RISCV64.HasZBA = isSet(v, RISCV_HWPROBE_EXT_ZBA)
+ RISCV64.HasZBB = isSet(v, RISCV_HWPROBE_EXT_ZBB)
+ RISCV64.HasZBS = isSet(v, RISCV_HWPROBE_EXT_ZBS)
+ }
+ if pairs[1].key != -1 {
+ v := pairs[1].value & RISCV_HWPROBE_MISALIGNED_MASK
+ RISCV64.HasMisalignedFast = v == RISCV_HWPROBE_MISALIGNED_FAST
+ }
+ }
+
+ // Let's double check with HWCAP if the C and V extensions do not appear to be supported.
+ // This may happen if we're running on an kernel earlier than 6.4.
+
+ if !RISCV64.HasC {
+ RISCV64.HasC = isSet(HWCap, hwcap_RISCV_ISA_C)
+ }
+
+ if !RISCV64.HasV {
+ RISCV64.HasV = isSet(HWCap, hwcap_RISCV_ISA_V)
+ }
+}
diff --git a/src/internal/cpu/cpu_riscv64_other.go b/src/internal/cpu/cpu_riscv64_other.go
new file mode 100644
index 0000000..f53b908
--- /dev/null
+++ b/src/internal/cpu/cpu_riscv64_other.go
@@ -0,0 +1,12 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build riscv64 && !linux
+
+package cpu
+
+func osInit() {
+ // Other operating systems do not support the hwprobe syscall or reading HWCap
+ // from auxiliary vector,
+}
diff --git a/src/runtime/os_linux_noauxv.go b/src/runtime/os_linux_noauxv.go
index ff37727..665e751 100644
--- a/src/runtime/os_linux_noauxv.go
+++ b/src/runtime/os_linux_noauxv.go
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-//go:build linux && !arm && !arm64 && !loong64 && !mips && !mipsle && !mips64 && !mips64le && !s390x && !ppc64 && !ppc64le
+//go:build linux && !arm && !arm64 && !loong64 && !mips && !mipsle && !mips64 && !mips64le && !s390x && !ppc64 && !ppc64le && !riscv64
package runtime
diff --git a/src/runtime/os_linux_riscv64.go b/src/runtime/os_linux_riscv64.go
index 9be88a5..af9f9ce 100644
--- a/src/runtime/os_linux_riscv64.go
+++ b/src/runtime/os_linux_riscv64.go
@@ -4,4 +4,36 @@
package runtime
+import (
+ "internal/cpu"
+ "runtime/internal/syscall"
+ "unsafe"
+)
+
+func archauxv(tag, val uintptr) {
+ switch tag {
+ case _AT_HWCAP:
+ cpu.HWCap = uint(val)
+ }
+}
+
func osArchInit() {}
+
+type hwprobePair = struct {
+ key int64
+ value uint64
+}
+
+//go:linkname internal_cpu_RISCVHWProbe internal/cpu.RISCVHWProbe
+func internal_cpu_RISCVHWProbe(pairs []hwprobePair, flags uint) bool {
+ // riscv_hwprobe's syscall number from /usr/include/riscv64-linux-gnu/asm/unistd.h
+ const riscvHWProbe uintptr = 258
+
+ if len(pairs) == 0 {
+ return false
+ }
+ // Passing in a cpuCount of 0 and a cpu of nil ensures that only extensions supported by all the
+ // cores are returned, which is the behaviour we want in internal/cpu.
+ _, _, e1 := syscall.Syscall6(riscvHWProbe, uintptr(unsafe.Pointer(&pairs[0])), uintptr(len(pairs)), uintptr(0), uintptr(unsafe.Pointer(nil)), uintptr(flags), 0)
+ return e1 == 0
+}
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Mark Ryan (Gerrit)
Mark Ryan uploaded patch set #2 to this change.
cpu/internal: provide runtime detection of RISC-V extensions on Linux
Add a RISCV64 variable to cpu/internal that indicates both the presence of RISC-V
extensions and performance information about the underlying RISC-V cores.
The variable is only populated with non false values on Linux. The detection code
first attempts to use the riscv_hwprobe syscall introduced in Linux 6.4, falling
back to HWCAP if riscv_hwprobe is not supported. The patch can detect the C, V,
Zba, Zbb and Zbs extensions. V, Zba, Zbb and Zbs can only be detected on a 6.5
kernel or later (without backports).
Updates #61416
Change-Id: I2d8289345c885b699afff441d417cae38f6bdc54
---
M src/internal/cpu/cpu.go
M src/internal/cpu/cpu_riscv64.go
A src/internal/cpu/cpu_riscv64_linux.go
A src/internal/cpu/cpu_riscv64_other.go
M src/runtime/os_linux_noauxv.go
M src/runtime/os_linux_riscv64.go
6 files changed, 164 insertions(+), 1 deletion(-)
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Mark Ryan (Gerrit)
Mark Ryan uploaded patch set #3 to this change.
Mark Ryan (Gerrit)
Attention is currently required from: Austin Clements, Joel Sing, Keith Randall, M Zhuo, Martin Möhrmann.
1 comment:
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Keith Randall (Gerrit)
Attention is currently required from: Austin Clements, Joel Sing, M Zhuo, Mark Ryan, Martin Möhrmann.
1 comment:
Patchset:
Normally we add stuff like this to x/sys/cpu first, then copy here just the bits needed by the stdlib itself.
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Mark Ryan (Gerrit)
Attention is currently required from: Austin Clements, Joel Sing, Keith Randall, M Zhuo, Martin Möhrmann.
1 comment:
Patchset:
Normally we add stuff like this to x/sys/cpu first, then copy here just the bits needed by the stdli […]
The x/sys/cpu patch is still a few weeks away as it's dependent on Linux 6.5 to generate the Zba, Zbb, Zbs and V constants for hwprobe. It will also build upon https://go-review.googlesource.com/c/sys/+/508676, which isn't merged yet. I'll update this patch, if necessary, when the x/sys/cpu patch is available.
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Martin Möhrmann (Gerrit)
Attention is currently required from: Austin Clements, Joel Sing, Keith Randall, M Zhuo, Mark Ryan.
1 comment:
File src/internal/cpu/cpu_riscv64.go:
On Linux we rely on the riscv_hwprobe syscall falling
// back to HWCAP where riscv_hwprobe is not available.
we usually use HWCAP first. any downside in first using HWCAP that should be available without making a syscall?
If everything we need in std lib/runtime can be checked with HWCAP than we can also avoid the complexity of adding and maintaining code for the syscall/fallback.
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Mark Ryan (Gerrit)
Attention is currently required from: Austin Clements, Joel Sing, Keith Randall, M Zhuo, Martin Möhrmann.
1 comment:
File src/internal/cpu/cpu_riscv64.go:
On Linux we rely on the riscv_hwprobe syscall falling
// back to HWCAP where riscv_hwprobe is not available.
we usually use HWCAP first. […]
HWCAP can only detect single letter extensions, such as V and C. It can't detect multi-letter extensions such as Zba, Zbb and Zbs. riscv64_hwprobe can detect both single and multi-letter extensions in addition to providing other information about the CPU that will be useful, e.g., whether fast misaligned accesses are supported. If we want to take advantage of current and future (e.g., vector crypto) multi-letter extensions in stdlib/runtime, we need the syscall.
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Martin Möhrmann (Gerrit)
Attention is currently required from: Austin Clements, Joel Sing, Keith Randall, M Zhuo, Mark Ryan.
1 comment:
File src/internal/cpu/cpu_riscv64.go:
On Linux we rely on the riscv_hwprobe syscall falling
// back to HWCAP where riscv_hwprobe is not available.
HWCAP can only detect single letter extensions, such as V and C. […]
ack. Then once we actually have use cases for these in runtime/std lib we can add the syscall. If for now we only need c and v at first I think HWCAP would suffice.
At the end depends what immediate use cases we have.
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Joel Sing (Gerrit)
Attention is currently required from: Austin Clements, Keith Randall, M Zhuo, Mark Ryan.
1 comment:
Patchset:
The x/sys/cpu patch is still a few weeks away as it's dependent on Linux 6. […]
As indicated, we should get working code into x/sys/cpu first - then some portion of that code is literally copied to internal/cpu (you can fetch the above CL and mail out a change that builds on top of that). Also, if the code is not able to go into x/sys/cpu, then it's probably not ready for internal/cpu either... that said, I'm fairly sure there will be prior art for working around the inability to generate constants (e.g. hardcoding for now, replacing with auto generated later).
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Mark Ryan (Gerrit)
Attention is currently required from: Austin Clements, Keith Randall, M Zhuo, Micheal Cooper.
1 comment:
File src/internal/cpu/cpu_riscv64_linux.go:
Patch Set #3, Line 78:
// This may happen if we're running on a kernel older than 6.4.If the kernel is older then 6.4, it does not support the RISCV_HWPROBE system call. […]
If the syscall is not implemented, RISCVHWProbe will return false and the body of the if statement above will not execute. We'll then fall down to here to double check to see whether 'V' and 'C' are available via hwcap. Note that I now think that we should remove the hwcap check for 'V' here as doing so would erroneously detect Vector 0.7 on some boards already on the market. This is presumably not what we want as 0.7 is not binary compatible with the ratified 1.0 version of Vector. See my comments here https://github.com/golang/go/issues/62238#issuecomment-1735404397.
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Micheal Cooper (Gerrit)
Attention is currently required from: Austin Clements, Keith Randall, M Zhuo, Mark Ryan.
1 comment:
File src/internal/cpu/cpu_riscv64_linux.go:
Patch Set #3, Line 78:
// This may happen if we're running on a kernel older than 6.4.
If the kernel is older then 6.4, it does not support the RISCV_HWPROBE system call. What happens when the kernel calls an unsupported system call? If an error occurs, the program may not reach this point.
To view, visit change 522995. To unsubscribe, or for help writing mail filters, visit settings.
Mark Ryan (Gerrit)
Mark Ryan uploaded new patchset![]( "Open in Gerrit")
- Austin Clements
- Keith Randall
- Meng Zhuo
- Micheal Cooper
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 added 3 comments![]( "Open in Gerrit")
Mark RyanNormally we add stuff like this to x/sys/cpu first, then copy here just the bits needed by the stdlib itself.
The x/sys/cpu patch is still a few weeks away as it's dependent on Linux 6.5 to generate the Zba, Zbb, Zbs and V constants for hwprobe. It will also build upon https://go-review.googlesource.com/c/sys/+/508676, which isn't merged yet. I'll update this patch, if necessary, when the x/sys/cpu patch is available.
As indicated, we should get working code into x/sys/cpu first - then some portion of that code is literally copied to internal/cpu (you can fetch the above CL and mail out a change that builds on top of that). Also, if the code is not able to go into x/sys/cpu, then it's probably not ready for internal/cpu either... that said, I'm fairly sure there will be prior art for working around the inability to generate constants (e.g. hardcoding for now, replacing with auto generated later).
The x/sys/cpu patch is still a few weeks away as it's dependent on Linux 6.5 to generate the Zba, Zbb, Zbs and V constants for hwprobe.
Turns out it was almost a year away. It did eventually get submitted and merged though.
// RISC-V doesn't have a 'cpuid' equivalent. On Linux we rely on the riscv_hwprobe syscall falling
// back to HWCAP where riscv_hwprobe is not available.we usually use HWCAP first. any downside in first using HWCAP that should be available without making a syscall?
If everything we need in std lib/runtime can be checked with HWCAP than we can also avoid the complexity of adding and maintaining code for the syscall/fallback.
HWCAP can only detect single letter extensions, such as V and C. It can't detect multi-letter extensions such as Zba, Zbb and Zbs. riscv64_hwprobe can detect both single and multi-letter extensions in addition to providing other information about the CPU that will be useful, e.g., whether fast misaligned accesses are supported. If we want to take advantage of current and future (e.g., vector crypto) multi-letter extensions in stdlib/runtime, we need the syscall.
ack. Then once we actually have use cases for these in runtime/std lib we can add the syscall. If for now we only need c and v at first I think HWCAP would suffice.
At the end depends what immediate use cases we have.
A very belated update here. It turns out that we cannot use hwcap to safely detect the presence of V (or rather RVV 1.0). There's a comment in cpu_riscv64_linux.go explaining why.
_, _, e1 := syscall.Syscall6(sys_RISCV_HWPROBE, uintptr(unsafe.Pointer(&pairs[0])), uintptr(len(pairs)), uintptr(0), uintptr(unsafe.Pointer(nil)), uintptr(flags), 0)There is a VDSO entry for riscv_hwprobe that should allow us to avoid the syscall entirely as, IIRC, it can handle the case where the caller only requests extensions that are supported on all cores, which is what we're doing here. I wasn't sure whether it was worth implementing the VDSO call though as we're only calling this once from osInit. However, I can have a go at implementing it if people think it's worth doing.
Related details
- Austin Clements
- Joel Sing
- Keith Randall
- Martin Möhrmann
- Meng Zhuo
- Micheal Cooper
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 4 comments![]( "Open in Gerrit")
Votes added by Joel Sing
| Code-Review | +2 |
4 comments
// RISC-V doesn't have a 'cpuid' equivalent. On Linux we rely on the riscv_hwprobe syscall falling
// back to HWCAP where riscv_hwprobe is not available.Mark Ryanwe usually use HWCAP first. any downside in first using HWCAP that should be available without making a syscall?
If everything we need in std lib/runtime can be checked with HWCAP than we can also avoid the complexity of adding and maintaining code for the syscall/fallback.
Martin MöhrmannHWCAP can only detect single letter extensions, such as V and C. It can't detect multi-letter extensions such as Zba, Zbb and Zbs. riscv64_hwprobe can detect both single and multi-letter extensions in addition to providing other information about the CPU that will be useful, e.g., whether fast misaligned accesses are supported. If we want to take advantage of current and future (e.g., vector crypto) multi-letter extensions in stdlib/runtime, we need the syscall.
Mark Ryanack. Then once we actually have use cases for these in runtime/std lib we can add the syscall. If for now we only need c and v at first I think HWCAP would suffice.
At the end depends what immediate use cases we have.
A very belated update here. It turns out that we cannot use hwcap to safely detect the presence of V (or rather RVV 1.0). There's a comment in cpu_riscv64_linux.go explaining why.
Acknowledged
//go:linkname riscvHWProbeIs linkname actually needed here?
// from auxiliary vector,```suggestion
// from auxiliary vector.
```
_, _, e1 := syscall.Syscall6(sys_RISCV_HWPROBE, uintptr(unsafe.Pointer(&pairs[0])), uintptr(len(pairs)), uintptr(0), uintptr(unsafe.Pointer(nil)), uintptr(flags), 0)There is a VDSO entry for riscv_hwprobe that should allow us to avoid the syscall entirely as, IIRC, it can handle the case where the caller only requests extensions that are supported on all cores, which is what we're doing here. I wasn't sure whether it was worth implementing the VDSO call though as we're only calling this once from osInit. However, I can have a go at implementing it if people think it's worth doing.
- Austin Clements
- Keith Randall
- Mark Ryan
- Martin Möhrmann
- Meng Zhuo
- Micheal Cooper
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 uploaded new patchset![]( "Open in Gerrit")
Related details
- Austin Clements
- Keith Randall
- Mark Ryan
- Martin Möhrmann
- Meng Zhuo
- Micheal Cooper
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 added 3 comments![]( "Open in Gerrit")
//go:linkname riscvHWProbeIs linkname actually needed here?
Yes, I think so, assuming I'm understanding the guidelines in https://github.com/golang/go/issues/67401 correctly.
```suggestion
// from auxiliary vector.
```
Done
_, _, e1 := syscall.Syscall6(sys_RISCV_HWPROBE, uintptr(unsafe.Pointer(&pairs[0])), uintptr(len(pairs)), uintptr(0), uintptr(unsafe.Pointer(nil)), uintptr(flags), 0)Joel SingThere is a VDSO entry for riscv_hwprobe that should allow us to avoid the syscall entirely as, IIRC, it can handle the case where the caller only requests extensions that are supported on all cores, which is what we're doing here. I wasn't sure whether it was worth implementing the VDSO call though as we're only calling this once from osInit. However, I can have a go at implementing it if people think it's worth doing.
I doubt it's worth it, at least for now. Perhaps add a TODO?
- Austin Clements
- Joel Sing
- Keith Randall
- Martin Möhrmann
- Meng Zhuo
- Micheal Cooper
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Meng Zhuo (Gerrit)
Meng Zhuo voted Code-Review+2![]( "Open in Gerrit")
- Austin Clements
- Joel Sing
- Keith Randall
- Mark Ryan
- Martin Möhrmann
- Micheal Cooper
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Martin Möhrmann (Gerrit)
Martin Möhrmann added 1 comment![]( "Open in Gerrit")
HasFastMisaligned bool // Fast misaligned accessesSince these are a few and we only add them to std library if they will be used can you confirm there are other CLs planed that intend to use these?
Related details
- Austin Clements
- Joel Sing
- Keith Randall
- Mark Ryan
- Micheal Cooper
Mark Ryan (Gerrit)
Mark Ryan added 1 comment![]( "Open in Gerrit")
HasFastMisaligned bool // Fast misaligned accessesSince these are a few and we only add them to std library if they will be used can you confirm there are other CLs planed that intend to use these?
I would say that we definitely need HasMisalignedFast and HasV. I think it's unlikely we would perform a dynamic runtime check for HasC or the bitmanip extensions (Zba, Zbb, Zbs) so I believe we can do without them.
I included HasC, HasZba, HasZbb and HasZbs so that the RISCV64 structure in the runtime matched the one in golang.org/x/sys/cpu. If the two structures (or rather the two sets of available options) are not in sync, the user may see warnings when using GODEBUG. For example, supposing we removed Zba from the list of options that can be detected by the runtime. An application that imported golang.org/x/sys/cpu and was run with GODEBUG=cpu.zba=off would produce a warning from the runtime (but not from golang.org/x/sys/cpu where the Zba option is already available), e.g.,
```
GODEBUG: unknown cpu feature "zba"
```
However, now that you raise the topic, I notice that the structures for the other architectures are not in sync, and also combining different versions of Go and golang.org/x/sys/cpu will likely produce the same effect for the foreseeable future (as more extensions are added), so perhaps this is a non issue, or an issue that should be addressed in a different way.
Just one more point of information here. The number of extensions detectable by riscv_hwprobe has increased dramatically in recent kernels. It can now detect over 50 extensions and some of these are things that will be useful to detect at runtime, e.g., vector crypto. So in the future there will be more candidate elements for this structure.
Related details
- Austin Clements
- Joel Sing
- Keith Randall
- Martin Möhrmann
- Micheal Cooper
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Martin Möhrmann (Gerrit)
Martin Möhrmann added 1 comment![]( "Open in Gerrit")
HasFastMisaligned bool // Fast misaligned accessesMark RyanSince these are a few and we only add them to std library if they will be used can you confirm there are other CLs planed that intend to use these?
I would say that we definitely need HasMisalignedFast and HasV. I think it's unlikely we would perform a dynamic runtime check for HasC or the bitmanip extensions (Zba, Zbb, Zbs) so I believe we can do without them.
I included HasC, HasZba, HasZbb and HasZbs so that the RISCV64 structure in the runtime matched the one in golang.org/x/sys/cpu. If the two structures (or rather the two sets of available options) are not in sync, the user may see warnings when using GODEBUG. For example, supposing we removed Zba from the list of options that can be detected by the runtime. An application that imported golang.org/x/sys/cpu and was run with GODEBUG=cpu.zba=off would produce a warning from the runtime (but not from golang.org/x/sys/cpu where the Zba option is already available), e.g.,
```
GODEBUG: unknown cpu feature "zba"
```However, now that you raise the topic, I notice that the structures for the other architectures are not in sync, and also combining different versions of Go and golang.org/x/sys/cpu will likely produce the same effect for the foreseeable future (as more extensions are added), so perhaps this is a non issue, or an issue that should be addressed in a different way.
Just one more point of information here. The number of extensions detectable by riscv_hwprobe has increased dramatically in recent kernels. It can now detect over 50 extensions and some of these are things that will be useful to detect at runtime, e.g., vector crypto. So in the future there will be more candidate elements for this structure.
I agree that if we want to avoid errors from GODEBUG for internal/cpu vs x/sys/cpu its likely better to change the behaviour across all architectures and there seems to be potential for improvements. If its just about suppressing error reporting we may introduce a silence option that suppresses them when options are not found.
I think there is also an argument that we dont want to expose options that actually dont change any behaviour anywhere in std lib or runtime to not mislead users.
I can see the argument of keeping structures e.g. for kernel aligned. If we have these I think its fine to have the structure definition in full but not actually define the options and set the capabilities internally ontop of the unused fields.
If its just check values e.g. check bit 9 of register foo then I think we can just define the ones actually needed (we do that for e.g. amd64) and expand as needed.
In general I think its good to keep internal/cpu lightweight to things actually used so we dont end up constantly expanding and maintaining it, increasing size of structures and adding to startup time (in the cases where discovery is actually heavy e.g. lots of cpuid, sysctl ...).
Related details
- Austin Clements
- Joel Sing
- Keith Randall
- Mark Ryan
- Martin Möhrmann
- Micheal Cooper
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 uploaded new patchset![]( "Open in Gerrit")
Related details
- Austin Clements
- Joel Sing
- Keith Randall
- Mark Ryan
- Martin Möhrmann
- Micheal Cooper
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 added 1 comment![]( "Open in Gerrit")
HasFastMisaligned bool // Fast misaligned accessesMark RyanSince these are a few and we only add them to std library if they will be used can you confirm there are other CLs planed that intend to use these?
Martin MöhrmannI would say that we definitely need HasMisalignedFast and HasV. I think it's unlikely we would perform a dynamic runtime check for HasC or the bitmanip extensions (Zba, Zbb, Zbs) so I believe we can do without them.
I included HasC, HasZba, HasZbb and HasZbs so that the RISCV64 structure in the runtime matched the one in golang.org/x/sys/cpu. If the two structures (or rather the two sets of available options) are not in sync, the user may see warnings when using GODEBUG. For example, supposing we removed Zba from the list of options that can be detected by the runtime. An application that imported golang.org/x/sys/cpu and was run with GODEBUG=cpu.zba=off would produce a warning from the runtime (but not from golang.org/x/sys/cpu where the Zba option is already available), e.g.,
```
GODEBUG: unknown cpu feature "zba"
```However, now that you raise the topic, I notice that the structures for the other architectures are not in sync, and also combining different versions of Go and golang.org/x/sys/cpu will likely produce the same effect for the foreseeable future (as more extensions are added), so perhaps this is a non issue, or an issue that should be addressed in a different way.
Just one more point of information here. The number of extensions detectable by riscv_hwprobe has increased dramatically in recent kernels. It can now detect over 50 extensions and some of these are things that will be useful to detect at runtime, e.g., vector crypto. So in the future there will be more candidate elements for this structure.
I agree that if we want to avoid errors from GODEBUG for internal/cpu vs x/sys/cpu its likely better to change the behaviour across all architectures and there seems to be potential for improvements. If its just about suppressing error reporting we may introduce a silence option that suppresses them when options are not found.
I think there is also an argument that we dont want to expose options that actually dont change any behaviour anywhere in std lib or runtime to not mislead users.
I can see the argument of keeping structures e.g. for kernel aligned. If we have these I think its fine to have the structure definition in full but not actually define the options and set the capabilities internally ontop of the unused fields.
If its just check values e.g. check bit 9 of register foo then I think we can just define the ones actually needed (we do that for e.g. amd64) and expand as needed.
In general I think its good to keep internal/cpu lightweight to things actually used so we dont end up constantly expanding and maintaining it, increasing size of structures and adding to startup time (in the cases where discovery is actually heavy e.g. lots of cpuid, sysctl ...).
I've removed everything apart from HasFastMisaligned and HasV, and in doing so was able to simplify the patch a little.
Related details
- Austin Clements
- Joel Sing
- Keith Randall
- Martin Möhrmann
- Martin Möhrmann
- Micheal Cooper
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
| Code-Review | +2 |
| Commit-Queue | +1 |
2 comments
HasFastMisaligned bool // Fast misaligned accessesMark RyanSince these are a few and we only add them to std library if they will be used can you confirm there are other CLs planed that intend to use these?
Martin MöhrmannI would say that we definitely need HasMisalignedFast and HasV. I think it's unlikely we would perform a dynamic runtime check for HasC or the bitmanip extensions (Zba, Zbb, Zbs) so I believe we can do without them.
I included HasC, HasZba, HasZbb and HasZbs so that the RISCV64 structure in the runtime matched the one in golang.org/x/sys/cpu. If the two structures (or rather the two sets of available options) are not in sync, the user may see warnings when using GODEBUG. For example, supposing we removed Zba from the list of options that can be detected by the runtime. An application that imported golang.org/x/sys/cpu and was run with GODEBUG=cpu.zba=off would produce a warning from the runtime (but not from golang.org/x/sys/cpu where the Zba option is already available), e.g.,
```
GODEBUG: unknown cpu feature "zba"
```However, now that you raise the topic, I notice that the structures for the other architectures are not in sync, and also combining different versions of Go and golang.org/x/sys/cpu will likely produce the same effect for the foreseeable future (as more extensions are added), so perhaps this is a non issue, or an issue that should be addressed in a different way.
Just one more point of information here. The number of extensions detectable by riscv_hwprobe has increased dramatically in recent kernels. It can now detect over 50 extensions and some of these are things that will be useful to detect at runtime, e.g., vector crypto. So in the future there will be more candidate elements for this structure.
Mark RyanI agree that if we want to avoid errors from GODEBUG for internal/cpu vs x/sys/cpu its likely better to change the behaviour across all architectures and there seems to be potential for improvements. If its just about suppressing error reporting we may introduce a silence option that suppresses them when options are not found.
I think there is also an argument that we dont want to expose options that actually dont change any behaviour anywhere in std lib or runtime to not mislead users.
I can see the argument of keeping structures e.g. for kernel aligned. If we have these I think its fine to have the structure definition in full but not actually define the options and set the capabilities internally ontop of the unused fields.
If its just check values e.g. check bit 9 of register foo then I think we can just define the ones actually needed (we do that for e.g. amd64) and expand as needed.
In general I think its good to keep internal/cpu lightweight to things actually used so we dont end up constantly expanding and maintaining it, increasing size of structures and adding to startup time (in the cases where discovery is actually heavy e.g. lots of cpuid, sysctl ...).
I've removed everything apart from HasFastMisaligned and HasV, and in doing so was able to simplify the patch a little.
Done
//go:linkname riscvHWProbeMark RyanIs linkname actually needed here?
Yes, I think so, assuming I'm understanding the guidelines in https://github.com/golang/go/issues/67401 correctly.
- Austin Clements
- Keith Randall
- Mark Ryan
- Martin Möhrmann
- Martin Möhrmann
- Micheal Cooper
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![]( "Open in Gerrit")
| Code-Review | +2 |
| Commit-Queue | +1 |
Related details
- Austin Clements
- Keith Randall
- Mark Ryan
- Martin Möhrmann
- Martin Möhrmann
- Micheal Cooper
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Chase (Gerrit)
David Chase voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Austin Clements
- Keith Randall
- Mark Ryan
- Martin Möhrmann
- Martin Möhrmann
- Micheal Cooper
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Knyszek (Gerrit)
Michael Knyszek voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Austin Clements
- Keith Randall
- Mark Ryan
- Martin Möhrmann
- Martin Möhrmann
- Micheal Cooper
Code-Review
No-Unresolved-Comments
No-Wait-Release
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joel Sing (Gerrit)
Joel Sing submitted the change![]( "Open in Gerrit")
Change information
cpu/internal: provide runtime detection of RISC-V extensions on Linux
Add a RISCV64 variable to cpu/internal that indicates both the presence
of RISC-V extensions and performance information about the underlying
RISC-V cores. The variable is only populated with non false values on
Linux. The detection code relies on the riscv_hwprobe syscall
introduced in Linux 6.4. The patch can detect RVV 1.0 and whether
the CPU supports fast misaligned accesses. It can only detect RVV 1.0
on a 6.5 kernel or later (without backports).
Updates #61416
- M src/internal/cpu/cpu.go
- M src/internal/cpu/cpu_riscv64.go
- A src/internal/cpu/cpu_riscv64_linux.go
- A src/internal/cpu/cpu_riscv64_other.go
- M src/runtime/os_linux_riscv64.go
Code-Review: +2 by Meng Zhuo, +1 by Michael Knyszek, +1 by David Chase, +2 by Joel Sing
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |