From a40188c715d591dbeaa5c200b864ab8f4c61a5a9 Mon Sep 17 00:00:00 2001 From: Nuno Cruces Date: Sat, 15 Feb 2025 00:17:29 +0000 Subject: [PATCH] compiler: fix ARMv8 regression introduced in #2345 (#2365) Signed-off-by: Nuno Cruces --- config.go | 11 ++++----- config_supported.go | 19 --------------- config_test.go | 6 +---- config_unsupported.go | 8 ------- .../engine/wazevo/backend/backend_test.go | 2 +- .../integration_test/engine/threads_test.go | 2 +- internal/platform/platform.go | 20 ++++++++++++---- internal/platform/platform_amd64.go | 7 ------ internal/platform/platform_arm64.go | 7 ------ internal/platform/platform_test.go | 22 ------------------ runtime.go | 23 +++++++++++++++++-- 11 files changed, 44 insertions(+), 83 deletions(-) delete mode 100644 config_supported.go delete mode 100644 config_unsupported.go delete mode 100644 internal/platform/platform_amd64.go delete mode 100644 internal/platform/platform_arm64.go delete mode 100644 internal/platform/platform_test.go diff --git a/config.go b/config.go index ea7b84f44..9fa2bada8 100644 --- a/config.go +++ b/config.go @@ -12,8 +12,6 @@ import ( "github.com/tetratelabs/wazero/api" experimentalsys "github.com/tetratelabs/wazero/experimental/sys" - "github.com/tetratelabs/wazero/internal/engine/interpreter" - "github.com/tetratelabs/wazero/internal/engine/wazevo" "github.com/tetratelabs/wazero/internal/filecache" "github.com/tetratelabs/wazero/internal/internalapi" "github.com/tetratelabs/wazero/internal/platform" @@ -175,7 +173,9 @@ type RuntimeConfig interface { // NewRuntimeConfig returns a RuntimeConfig using the compiler if it is supported in this environment, // or the interpreter otherwise. func NewRuntimeConfig() RuntimeConfig { - return newRuntimeConfig() + ret := engineLessConfig.clone() + ret.engineKind = engineKindAuto + return ret } type newEngine func(context.Context, api.CoreFeatures, filecache.Cache) wasm.Engine @@ -203,7 +203,8 @@ var engineLessConfig = &runtimeConfig{ type engineKind int const ( - engineKindCompiler engineKind = iota + engineKindAuto engineKind = iota - 1 + engineKindCompiler engineKindInterpreter engineKindCount ) @@ -234,7 +235,6 @@ const ( func NewRuntimeConfigCompiler() RuntimeConfig { ret := engineLessConfig.clone() ret.engineKind = engineKindCompiler - ret.newEngine = wazevo.NewEngine return ret } @@ -242,7 +242,6 @@ func NewRuntimeConfigCompiler() RuntimeConfig { func NewRuntimeConfigInterpreter() RuntimeConfig { ret := engineLessConfig.clone() ret.engineKind = engineKindInterpreter - ret.newEngine = interpreter.NewEngine return ret } diff --git a/config_supported.go b/config_supported.go deleted file mode 100644 index 214c2bb8c..000000000 --- a/config_supported.go +++ /dev/null @@ -1,19 +0,0 @@ -// Note: The build constraints here are about the compiler, which is more -// narrow than the architectures supported by the assembler. -// -// Constraints here must match platform.CompilerSupported. -// -// Meanwhile, users who know their runtime.GOOS can operate with the compiler -// may choose to use NewRuntimeConfigCompiler explicitly. -//go:build (amd64 || arm64) && (linux || darwin || freebsd || netbsd || dragonfly || solaris || windows) - -package wazero - -import "github.com/tetratelabs/wazero/internal/platform" - -func newRuntimeConfig() RuntimeConfig { - if platform.CompilerSupported() { - return NewRuntimeConfigCompiler() - } - return NewRuntimeConfigInterpreter() -} diff --git a/config_test.go b/config_test.go index 1b58aa61a..490e1bf29 100644 --- a/config_test.go +++ b/config_test.go @@ -670,9 +670,5 @@ func TestNewRuntimeConfig(t *testing.T) { // Should be cloned from the source. require.NotEqual(t, engineLessConfig, c) // Ensures if the correct engine is selected. - if platform.CompilerSupported() { - require.Equal(t, engineKindCompiler, c.engineKind) - } else { - require.Equal(t, engineKindInterpreter, c.engineKind) - } + require.Equal(t, engineKindAuto, c.engineKind) } diff --git a/config_unsupported.go b/config_unsupported.go deleted file mode 100644 index be56a4bc2..000000000 --- a/config_unsupported.go +++ /dev/null @@ -1,8 +0,0 @@ -// This is the opposite constraint of config_supported.go -//go:build !(amd64 || arm64) || !(linux || darwin || freebsd || netbsd || dragonfly || solaris || windows) - -package wazero - -func newRuntimeConfig() RuntimeConfig { - return NewRuntimeConfigInterpreter() -} diff --git a/internal/engine/wazevo/backend/backend_test.go b/internal/engine/wazevo/backend/backend_test.go index 7153e2425..7c3b9dd22 100644 --- a/internal/engine/wazevo/backend/backend_test.go +++ b/internal/engine/wazevo/backend/backend_test.go @@ -22,7 +22,7 @@ import ( ) func TestMain(m *testing.M) { - if !platform.CompilerSupported() { + if !platform.CompilerSupports(api.CoreFeaturesV2 | experimental.CoreFeaturesThreads) { os.Exit(0) } os.Exit(m.Run()) diff --git a/internal/integration_test/engine/threads_test.go b/internal/integration_test/engine/threads_test.go index d075e3db8..3baa8c73f 100644 --- a/internal/integration_test/engine/threads_test.go +++ b/internal/integration_test/engine/threads_test.go @@ -54,7 +54,7 @@ func TestThreadsNotEnabled(t *testing.T) { } func TestThreadsCompiler_hammer(t *testing.T) { - if !platform.CompilerSupported() { + if !platform.CompilerSupports(api.CoreFeaturesV2 | experimental.CoreFeaturesThreads) { t.Skip() } runAllTests(t, threadTests, wazero.NewRuntimeConfigCompiler().WithCoreFeatures(api.CoreFeaturesV2|experimental.CoreFeaturesThreads), false) diff --git a/internal/platform/platform.go b/internal/platform/platform.go index b9af094c1..532cc7b8c 100644 --- a/internal/platform/platform.go +++ b/internal/platform/platform.go @@ -6,18 +6,28 @@ package platform import ( "runtime" -) -// archRequirementsVerified is set by platform-specific init to true if the platform is supported -var archRequirementsVerified bool + "github.com/tetratelabs/wazero/api" + "github.com/tetratelabs/wazero/experimental" +) // CompilerSupported includes constraints here and also the assembler. func CompilerSupported() bool { + return CompilerSupports(api.CoreFeaturesV2) +} + +func CompilerSupports(features api.CoreFeatures) bool { switch runtime.GOOS { case "linux", "darwin", "freebsd", "netbsd", "dragonfly", "windows": - return archRequirementsVerified + if runtime.GOARCH == "arm64" { + if features.IsEnabled(experimental.CoreFeaturesThreads) { + return CpuFeatures.Has(CpuFeatureArm64Atomic) + } + return true + } + fallthrough case "solaris", "illumos": - return runtime.GOARCH == "amd64" && archRequirementsVerified + return runtime.GOARCH == "amd64" && CpuFeatures.Has(CpuFeatureAmd64SSE4_1) default: return false } diff --git a/internal/platform/platform_amd64.go b/internal/platform/platform_amd64.go deleted file mode 100644 index 59aaf5eae..000000000 --- a/internal/platform/platform_amd64.go +++ /dev/null @@ -1,7 +0,0 @@ -package platform - -// init verifies that the current CPU supports the required AMD64 instructions -func init() { - // Ensure SSE4.1 is supported. - archRequirementsVerified = CpuFeatures.Has(CpuFeatureAmd64SSE4_1) -} diff --git a/internal/platform/platform_arm64.go b/internal/platform/platform_arm64.go deleted file mode 100644 index a8df707c7..000000000 --- a/internal/platform/platform_arm64.go +++ /dev/null @@ -1,7 +0,0 @@ -package platform - -// init verifies that the current CPU supports the required ARM64 features -func init() { - // Ensure atomic instructions are supported. - archRequirementsVerified = CpuFeatures.Has(CpuFeatureArm64Atomic) -} diff --git a/internal/platform/platform_test.go b/internal/platform/platform_test.go deleted file mode 100644 index 8fce9fa81..000000000 --- a/internal/platform/platform_test.go +++ /dev/null @@ -1,22 +0,0 @@ -package platform - -import ( - "runtime" - "testing" - - "github.com/tetratelabs/wazero/internal/testing/require" -) - -func Test_archRequirementsVerified(t *testing.T) { - switch runtime.GOARCH { - case "arm64": - require.True(t, archRequirementsVerified) - case "amd64": - // TODO: once we find a way to test no SSE4 platform, use build tag and choose the correct assertion. - // For now, we assume that all the amd64 machine we are testing are with SSE 4 to avoid - // accidentally turn off compiler on the modern amd64 platform. - require.True(t, archRequirementsVerified) - default: - require.False(t, archRequirementsVerified) - } -} diff --git a/runtime.go b/runtime.go index 34742289e..f0d17d2a8 100644 --- a/runtime.go +++ b/runtime.go @@ -7,7 +7,10 @@ import ( "github.com/tetratelabs/wazero/api" experimentalapi "github.com/tetratelabs/wazero/experimental" + "github.com/tetratelabs/wazero/internal/engine/interpreter" + "github.com/tetratelabs/wazero/internal/engine/wazevo" "github.com/tetratelabs/wazero/internal/expctxkeys" + "github.com/tetratelabs/wazero/internal/platform" internalsock "github.com/tetratelabs/wazero/internal/sock" internalsys "github.com/tetratelabs/wazero/internal/sys" "github.com/tetratelabs/wazero/internal/wasm" @@ -148,15 +151,31 @@ func NewRuntime(ctx context.Context) Runtime { // NewRuntimeWithConfig returns a runtime with the given configuration. func NewRuntimeWithConfig(ctx context.Context, rConfig RuntimeConfig) Runtime { config := rConfig.(*runtimeConfig) + configKind := config.engineKind + configEngine := config.newEngine + if configKind == engineKindAuto { + if platform.CompilerSupports(config.enabledFeatures) { + configKind = engineKindCompiler + } else { + configKind = engineKindInterpreter + } + } + if configEngine == nil { + if configKind == engineKindCompiler { + configEngine = wazevo.NewEngine + } else { + configEngine = interpreter.NewEngine + } + } var engine wasm.Engine var cacheImpl *cache if c := config.cache; c != nil { // If the Cache is configured, we share the engine. cacheImpl = c.(*cache) - engine = cacheImpl.initEngine(config.engineKind, config.newEngine, ctx, config.enabledFeatures) + engine = cacheImpl.initEngine(configKind, configEngine, ctx, config.enabledFeatures) } else { // Otherwise, we create a new engine. - engine = config.newEngine(ctx, config.enabledFeatures, nil) + engine = configEngine(ctx, config.enabledFeatures, nil) } store := wasm.NewStore(config.enabledFeatures, engine) return &runtime{