From 29b9cd4f762bb2a6225d5ee63c004e8fc91bde12 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 28 May 2024 10:49:04 +0900 Subject: [PATCH 1/3] cache: includes cpu features in cache keys there's a discussion about "offline" compilation, and if that happens, the CPU feature incompatibility becomes the problem. This hardens the cache against it by including the cpu id in the file cache keys so that the content will be used exactly on the same CPUs. Note that this impacts only amd64 target. Signed-off-by: Takeshi Yoneda --- internal/engine/wazevo/backend/isa/amd64/machine_test.go | 3 +++ internal/engine/wazevo/engine_cache.go | 8 ++++++++ internal/platform/cpuid.go | 2 ++ internal/platform/cpuid_amd64.go | 5 ++++- internal/platform/cpuid_unsupported.go | 3 +++ 5 files changed, 20 insertions(+), 1 deletion(-) diff --git a/internal/engine/wazevo/backend/isa/amd64/machine_test.go b/internal/engine/wazevo/backend/isa/amd64/machine_test.go index 5dd05081c6..18ad0a27ac 100644 --- a/internal/engine/wazevo/backend/isa/amd64/machine_test.go +++ b/internal/engine/wazevo/backend/isa/amd64/machine_test.go @@ -458,3 +458,6 @@ func (f *mockCpuFlags) Has(flag platform.CpuFeature) bool { func (f *mockCpuFlags) HasExtra(flag platform.CpuFeature) bool { return (f.extraFlags & flag) != 0 } + +// Raw implements the method of the same name in platform.CpuFeatureFlags. +func (f *mockCpuFlags) Raw() [2]uint64 { return [2]uint64{uint64(f.flags), uint64(f.extraFlags)} } diff --git a/internal/engine/wazevo/engine_cache.go b/internal/engine/wazevo/engine_cache.go index f7c0450aed..7b4faa7ceb 100644 --- a/internal/engine/wazevo/engine_cache.go +++ b/internal/engine/wazevo/engine_cache.go @@ -31,6 +31,14 @@ func fileCacheKey(m *wasm.Module) (ret filecache.Key) { s := sha256.New() s.Write(m.ID[:]) s.Write(magic) + // Write the CPU features so that we can cache the compiled module for the same CPU. + // This prevents the incompatible CPU features from being used. + cpu := platform.CpuFeatures.Raw() + // Reuse the `ret` buffer to write the first 16 bytes of the CPU features so that we can avoid the allocation. + binary.LittleEndian.PutUint64(ret[:8], cpu[0]) + binary.LittleEndian.PutUint64(ret[8:16], cpu[1]) + s.Write(ret[:16]) + // Finally, write the hash to the ret buffer. s.Sum(ret[:0]) return } diff --git a/internal/platform/cpuid.go b/internal/platform/cpuid.go index 25d7d3fdca..db457b1c0f 100644 --- a/internal/platform/cpuid.go +++ b/internal/platform/cpuid.go @@ -6,6 +6,8 @@ type CpuFeatureFlags interface { Has(cpuFeature CpuFeature) bool // HasExtra returns true when the specified extraFlag (represented as uint64) is supported HasExtra(cpuFeature CpuFeature) bool + // Raw returns the raw flags as a pair of uint64 values. This can be used for cache keying. + Raw() [2]uint64 } type CpuFeature uint64 diff --git a/internal/platform/cpuid_amd64.go b/internal/platform/cpuid_amd64.go index 8c9f1a9f34..a0bc98014e 100644 --- a/internal/platform/cpuid_amd64.go +++ b/internal/platform/cpuid_amd64.go @@ -3,7 +3,7 @@ package platform // CpuFeatures exposes the capabilities for this CPU, queried via the Has, HasExtra methods -var CpuFeatures CpuFeatureFlags = loadCpuFeatureFlags() +var CpuFeatures = loadCpuFeatureFlags() // cpuFeatureFlags implements CpuFeatureFlags interface type cpuFeatureFlags struct { @@ -57,3 +57,6 @@ func (f *cpuFeatureFlags) Has(cpuFeature CpuFeature) bool { func (f *cpuFeatureFlags) HasExtra(cpuFeature CpuFeature) bool { return (f.extraFlags & uint64(cpuFeature)) != 0 } + +// Raw implements the same method on the CpuFeatureFlags interface +func (f *cpuFeatureFlags) Raw() [2]uint64 { return [2]uint64{f.flags, f.extraFlags} } diff --git a/internal/platform/cpuid_unsupported.go b/internal/platform/cpuid_unsupported.go index 8ae826d367..bfd255b780 100644 --- a/internal/platform/cpuid_unsupported.go +++ b/internal/platform/cpuid_unsupported.go @@ -12,3 +12,6 @@ func (c *cpuFeatureFlags) Has(cpuFeature CpuFeature) bool { return false } // HasExtra implements the same method on the CpuFeatureFlags interface func (c *cpuFeatureFlags) HasExtra(cpuFeature CpuFeature) bool { return false } + +// Raw implements the same method on the CpuFeatureFlags interface +func (c *cpuFeatureFlags) Raw() [2]uint64 { return [2]uint64{0, 0} } From 7cedae981e5e9e1321b53c7106842d48e6ea8d8b Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 28 May 2024 11:00:04 +0900 Subject: [PATCH 2/3] nit: periods Signed-off-by: Takeshi Yoneda --- .../wazevo/backend/isa/amd64/machine_test.go | 6 +++--- internal/platform/cpuid_amd64.go | 16 ++++++++-------- internal/platform/cpuid_unsupported.go | 8 ++++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/engine/wazevo/backend/isa/amd64/machine_test.go b/internal/engine/wazevo/backend/isa/amd64/machine_test.go index 18ad0a27ac..78919ef7a7 100644 --- a/internal/engine/wazevo/backend/isa/amd64/machine_test.go +++ b/internal/engine/wazevo/backend/isa/amd64/machine_test.go @@ -443,18 +443,18 @@ L2: } } -// mockCpuFlags implements platform.CpuFeatureFlags +// mockCpuFlags implements platform.CpuFeatureFlags. type mockCpuFlags struct { flags platform.CpuFeature extraFlags platform.CpuFeature } -// Has implements the method of the same name in platform.CpuFeatureFlags +// Has implements the method of the same name in platform.CpuFeatureFlags. func (f *mockCpuFlags) Has(flag platform.CpuFeature) bool { return (f.flags & flag) != 0 } -// HasExtra implements the method of the same name in platform.CpuFeatureFlags +// HasExtra implements the method of the same name in platform.CpuFeatureFlags. func (f *mockCpuFlags) HasExtra(flag platform.CpuFeature) bool { return (f.extraFlags & flag) != 0 } diff --git a/internal/platform/cpuid_amd64.go b/internal/platform/cpuid_amd64.go index a0bc98014e..bf72f148d2 100644 --- a/internal/platform/cpuid_amd64.go +++ b/internal/platform/cpuid_amd64.go @@ -2,10 +2,10 @@ package platform -// CpuFeatures exposes the capabilities for this CPU, queried via the Has, HasExtra methods +// CpuFeatures exposes the capabilities for this CPU, queried via the Has, HasExtra methods. var CpuFeatures = loadCpuFeatureFlags() -// cpuFeatureFlags implements CpuFeatureFlags interface +// cpuFeatureFlags implements CpuFeatureFlags interface. type cpuFeatureFlags struct { flags uint64 extraFlags uint64 @@ -15,13 +15,13 @@ type cpuFeatureFlags struct { // implemented in impl_amd64.s func cpuid(arg1, arg2 uint32) (eax, ebx, ecx, edx uint32) -// cpuidAsBitmap combines the result of invoking cpuid to uint64 bitmap +// cpuidAsBitmap combines the result of invoking cpuid to uint64 bitmap. func cpuidAsBitmap(arg1, arg2 uint32) uint64 { _ /* eax */, _ /* ebx */, ecx, edx := cpuid(arg1, arg2) return (uint64(edx) << 32) | uint64(ecx) } -// loadStandardRange load flags from the standard range, panics otherwise +// loadStandardRange load flags from the standard range, panics otherwise. func loadStandardRange(id uint32) uint64 { // ensure that the id is in the valid range, returned by cpuid(0,0) maxRange, _, _, _ := cpuid(0, 0) @@ -31,7 +31,7 @@ func loadStandardRange(id uint32) uint64 { return cpuidAsBitmap(id, 0) } -// loadStandardRange load flags from the extended range, panics otherwise +// loadStandardRange load flags from the extended range, panics otherwise. func loadExtendedRange(id uint32) uint64 { // ensure that the id is in the valid range, returned by cpuid(0x80000000,0) maxRange, _, _, _ := cpuid(0x80000000, 0) @@ -48,15 +48,15 @@ func loadCpuFeatureFlags() CpuFeatureFlags { } } -// Has implements the same method on the CpuFeatureFlags interface +// Has implements the same method on the CpuFeatureFlags interface. func (f *cpuFeatureFlags) Has(cpuFeature CpuFeature) bool { return (f.flags & uint64(cpuFeature)) != 0 } -// HasExtra implements the same method on the CpuFeatureFlags interface +// HasExtra implements the same method on the CpuFeatureFlags interface. func (f *cpuFeatureFlags) HasExtra(cpuFeature CpuFeature) bool { return (f.extraFlags & uint64(cpuFeature)) != 0 } -// Raw implements the same method on the CpuFeatureFlags interface +// Raw implements the same method on the CpuFeatureFlags interface. func (f *cpuFeatureFlags) Raw() [2]uint64 { return [2]uint64{f.flags, f.extraFlags} } diff --git a/internal/platform/cpuid_unsupported.go b/internal/platform/cpuid_unsupported.go index bfd255b780..aef921af10 100644 --- a/internal/platform/cpuid_unsupported.go +++ b/internal/platform/cpuid_unsupported.go @@ -4,14 +4,14 @@ package platform var CpuFeatures CpuFeatureFlags = &cpuFeatureFlags{} -// cpuFeatureFlags implements CpuFeatureFlags for unsupported platforms +// cpuFeatureFlags implements CpuFeatureFlags for unsupported platforms. type cpuFeatureFlags struct{} -// Has implements the same method on the CpuFeatureFlags interface +// Has implements the same method on the CpuFeatureFlags interface. func (c *cpuFeatureFlags) Has(cpuFeature CpuFeature) bool { return false } -// HasExtra implements the same method on the CpuFeatureFlags interface +// HasExtra implements the same method on the CpuFeatureFlags interface. func (c *cpuFeatureFlags) HasExtra(cpuFeature CpuFeature) bool { return false } -// Raw implements the same method on the CpuFeatureFlags interface +// Raw implements the same method on the CpuFeatureFlags interface. func (c *cpuFeatureFlags) Raw() [2]uint64 { return [2]uint64{0, 0} } From 3de5bcb59d0bf62a2ef295b42fb866843bbc8612 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 28 May 2024 17:21:29 +0900 Subject: [PATCH 3/3] review: only take into account features we care about Signed-off-by: Takeshi Yoneda --- .../wazevo/backend/isa/amd64/machine_test.go | 2 +- internal/engine/wazevo/engine_cache.go | 7 +++---- internal/platform/cpuid.go | 7 +++++-- internal/platform/cpuid_amd64.go | 19 ++++++++++++++++++- internal/platform/cpuid_amd64_test.go | 12 ++++++++++++ internal/platform/cpuid_unsupported.go | 2 +- 6 files changed, 40 insertions(+), 9 deletions(-) diff --git a/internal/engine/wazevo/backend/isa/amd64/machine_test.go b/internal/engine/wazevo/backend/isa/amd64/machine_test.go index 78919ef7a7..b870ae4b6d 100644 --- a/internal/engine/wazevo/backend/isa/amd64/machine_test.go +++ b/internal/engine/wazevo/backend/isa/amd64/machine_test.go @@ -460,4 +460,4 @@ func (f *mockCpuFlags) HasExtra(flag platform.CpuFeature) bool { } // Raw implements the method of the same name in platform.CpuFeatureFlags. -func (f *mockCpuFlags) Raw() [2]uint64 { return [2]uint64{uint64(f.flags), uint64(f.extraFlags)} } +func (f *mockCpuFlags) Raw() uint64 { return 0 } diff --git a/internal/engine/wazevo/engine_cache.go b/internal/engine/wazevo/engine_cache.go index 7b4faa7ceb..e49353dc8e 100644 --- a/internal/engine/wazevo/engine_cache.go +++ b/internal/engine/wazevo/engine_cache.go @@ -34,10 +34,9 @@ func fileCacheKey(m *wasm.Module) (ret filecache.Key) { // Write the CPU features so that we can cache the compiled module for the same CPU. // This prevents the incompatible CPU features from being used. cpu := platform.CpuFeatures.Raw() - // Reuse the `ret` buffer to write the first 16 bytes of the CPU features so that we can avoid the allocation. - binary.LittleEndian.PutUint64(ret[:8], cpu[0]) - binary.LittleEndian.PutUint64(ret[8:16], cpu[1]) - s.Write(ret[:16]) + // Reuse the `ret` buffer to write the first 8 bytes of the CPU features so that we can avoid the allocation. + binary.LittleEndian.PutUint64(ret[:8], cpu) + s.Write(ret[:8]) // Finally, write the hash to the ret buffer. s.Sum(ret[:0]) return diff --git a/internal/platform/cpuid.go b/internal/platform/cpuid.go index db457b1c0f..0dc6ec19ce 100644 --- a/internal/platform/cpuid.go +++ b/internal/platform/cpuid.go @@ -6,8 +6,9 @@ type CpuFeatureFlags interface { Has(cpuFeature CpuFeature) bool // HasExtra returns true when the specified extraFlag (represented as uint64) is supported HasExtra(cpuFeature CpuFeature) bool - // Raw returns the raw flags as a pair of uint64 values. This can be used for cache keying. - Raw() [2]uint64 + // Raw returns the raw bitset that represents CPU features used by wazero. This can be used for cache keying. + // For now, we only use four features, so uint64 is enough. + Raw() uint64 } type CpuFeature uint64 @@ -19,9 +20,11 @@ const ( CpuFeatureAmd64SSE4_1 CpuFeature = 1 << 19 // CpuFeatureAmd64SSE4_2 is the flag to query CpuFeatureFlags.Has for SSEv4.2 capabilities on amd64 CpuFeatureAmd64SSE4_2 CpuFeature = 1 << 20 + // Note: when adding new features, ensure that the feature is included in CpuFeatureFlags.Raw. ) const ( // CpuExtraFeatureAmd64ABM is the flag to query CpuFeatureFlags.HasExtra for Advanced Bit Manipulation capabilities (e.g. LZCNT) on amd64 CpuExtraFeatureAmd64ABM CpuFeature = 1 << 5 + // Note: when adding new features, ensure that the feature is included in CpuFeatureFlags.Raw. ) diff --git a/internal/platform/cpuid_amd64.go b/internal/platform/cpuid_amd64.go index bf72f148d2..fbdb539366 100644 --- a/internal/platform/cpuid_amd64.go +++ b/internal/platform/cpuid_amd64.go @@ -59,4 +59,21 @@ func (f *cpuFeatureFlags) HasExtra(cpuFeature CpuFeature) bool { } // Raw implements the same method on the CpuFeatureFlags interface. -func (f *cpuFeatureFlags) Raw() [2]uint64 { return [2]uint64{f.flags, f.extraFlags} } +func (f *cpuFeatureFlags) Raw() uint64 { + // Below, we only set the first 4 bits for the features we care about, + // instead of setting all the unnecessary bits obtained from the CPUID instruction. + var ret uint64 + if f.Has(CpuFeatureAmd64SSE3) { + ret = 1 << 0 + } + if f.Has(CpuFeatureAmd64SSE4_1) { + ret |= 1 << 1 + } + if f.Has(CpuFeatureAmd64SSE4_2) { + ret |= 1 << 2 + } + if f.HasExtra(CpuExtraFeatureAmd64ABM) { + ret |= 1 << 3 + } + return ret +} diff --git a/internal/platform/cpuid_amd64_test.go b/internal/platform/cpuid_amd64_test.go index 001914fee2..626f63e557 100644 --- a/internal/platform/cpuid_amd64_test.go +++ b/internal/platform/cpuid_amd64_test.go @@ -16,3 +16,15 @@ func TestAmd64CpuId_cpuHasFeature(t *testing.T) { require.True(t, flags.HasExtra(CpuExtraFeatureAmd64ABM)) require.False(t, flags.HasExtra(1<<6)) // some other value } + +func TestAmd64CpuFeatureFlags_Raw(t *testing.T) { + flags := cpuFeatureFlags{ + flags: uint64(CpuFeatureAmd64SSE3 | CpuFeatureAmd64SSE4_1 | CpuFeatureAmd64SSE4_2), + extraFlags: uint64(CpuExtraFeatureAmd64ABM), + } + require.Equal(t, uint64(0b1111), flags.Raw()) + flags.flags = 0 + require.Equal(t, uint64(0b1000), flags.Raw()) + flags.extraFlags = 0 + require.Equal(t, uint64(0), flags.Raw()) +} diff --git a/internal/platform/cpuid_unsupported.go b/internal/platform/cpuid_unsupported.go index aef921af10..291bcea65f 100644 --- a/internal/platform/cpuid_unsupported.go +++ b/internal/platform/cpuid_unsupported.go @@ -14,4 +14,4 @@ func (c *cpuFeatureFlags) Has(cpuFeature CpuFeature) bool { return false } func (c *cpuFeatureFlags) HasExtra(cpuFeature CpuFeature) bool { return false } // Raw implements the same method on the CpuFeatureFlags interface. -func (c *cpuFeatureFlags) Raw() [2]uint64 { return [2]uint64{0, 0} } +func (c *cpuFeatureFlags) Raw() uint64 { return 0 }