From bf7b5755cc973fb9426bf498a9eaaffa339469bc Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 20 Jul 2026 02:09:27 -0400 Subject: [PATCH] [Refactor] (ShaderTranspiler, MG_Backend, MG_Util): gate the subgroup prefix-scan rewrite behind a generic device-quirk registry with GPU vendor detection and MOBILEGL_QUIRK_SUBGROUP_PREFIX_SCAN override, warn on template mismatch, and block ARB/NV subgroup spellings --- MobileGL/Config.h | 13 +++ MobileGL/ConfigLoader.cpp | 12 +++ MobileGL/MG_Backend/BackendObject.h | 16 +++ .../DirectGLES/BackendObject_DirectGLES.cpp | 26 +++++ .../BackendObject_DirectVulkan.cpp | 27 +++++ MobileGL/MG_Test/Program/ProgramUtilTest.cpp | 22 ++++ .../MG_Util/BackendLoaders/Vulkan/Loader.cpp | 2 + .../MG_Util/BackendLoaders/Vulkan/Loader.h | 2 + .../ShaderSourceProcessor.cpp | 102 ++++++++++++++++-- .../ShaderTranspiler/ShaderSourceProcessor.h | 6 +- 10 files changed, 219 insertions(+), 9 deletions(-) diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 04d08f33..e8715b67 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -20,6 +20,15 @@ namespace MobileGL::MG_Config { extern BackendType ActiveBackendType; + // Tri-state override for device-specific quirks: Auto lets the detected device decide, + // ForceOn/ForceOff bypass the detection in either direction. ForceOn only bypasses the + // device gate - each quirk keeps its structural safety checks. + enum class QuirkOverride : Uint8 { + Auto = 0, + ForceOn, + ForceOff, + }; + // Feature toggles parsed once from environment variables in MG_ConfigLoader::Init() // (ConfigLoader.cpp), before the accepted-env map is destroyed. All Bool fields share // one truthy rule: the variable is set, non-empty, not "0", and not "false" @@ -67,6 +76,10 @@ namespace MobileGL::MG_Config { // explicitly request a core profile via EGL_CONTEXT_OPENGL_PROFILE_MASK / a >=3.1 // version request. Bool RelaxedSemantics = false; + // MOBILEGL_QUIRK_SUBGROUP_PREFIX_SCAN: overrides the shader-source quirk that + // rewrites the recognized workgroup prefix-scan template on Qualcomm devices with + // subgroups wider than 32 lanes (see ShaderSourceProcessor's quirk registry). + QuirkOverride SubgroupPrefixScanQuirk = QuirkOverride::Auto; }; extern FeaturesTable Features; } // namespace MobileGL::MG_Config diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index f231bbc3..00a3f9b4 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -86,6 +86,17 @@ namespace MobileGL::MG_ConfigLoader { return it != acceptedEnvVariablesMap->end() && IsTruthyValue(it->second); } + // Quirk overrides are tri-state: an unset variable keeps device auto-detection, a truthy + // value forces the quirk on, anything else set ("0", "false", "") forces it off. + inline MG_Config::QuirkOverride QueryEnvQuirkOverride(const String& key) { + auto it = acceptedEnvVariablesMap->find(key); + if (it == acceptedEnvVariablesMap->end()) { + return MG_Config::QuirkOverride::Auto; + } + return IsTruthyValue(it->second) ? MG_Config::QuirkOverride::ForceOn + : MG_Config::QuirkOverride::ForceOff; + } + inline Uint32 QueryEnvUint32(const String& key, Uint32 defaultValue, Uint32 minValue, Uint32 maxValue) { auto it = acceptedEnvVariablesMap->find(key); if (it == acceptedEnvVariablesMap->end()) { @@ -123,6 +134,7 @@ namespace MobileGL::MG_ConfigLoader { features.TraceSkipAutodestroy = QueryEnvFlag("MOBILEGL_TRACE_SKIP_AUTODESTROY"); features.DisableUboRing = QueryEnvFlag("MOBILEGL_DISABLE_UBO_RING"); features.RelaxedSemantics = QueryEnvFlag("MOBILEGL_RELAXED_SEMANTICS"); + features.SubgroupPrefixScanQuirk = QueryEnvQuirkOverride("MOBILEGL_QUIRK_SUBGROUP_PREFIX_SCAN"); } inline void InitBackendType() { diff --git a/MobileGL/MG_Backend/BackendObject.h b/MobileGL/MG_Backend/BackendObject.h index 8c4abda9..f5e61501 100644 --- a/MobileGL/MG_Backend/BackendObject.h +++ b/MobileGL/MG_Backend/BackendObject.h @@ -230,6 +230,21 @@ namespace MobileGL { void (*SetSwapInterval)(Int interval); }; + // Coarse GPU vendor identity for gating device-specific quirks. Detected from the + // Vulkan physical-device vendorID or the GLES GL_VENDOR/GL_RENDERER strings; stays + // Unknown when detection is inconclusive, in which case auto-gated quirks stay off. + enum class GpuVendorKind : Uint8 { + Unknown = 0, + Qualcomm, + Arm, + Nvidia, + Amd, + Intel, + ImgTec, + // Software rasterizers (llvmpipe/lavapipe, SwiftShader). + Software, + }; + struct DynamicBackendParameters { SizeT UniformBufferOffsetAlignment = 256; // GL_MAX_TEXTURE_MAX_ANISOTROPY_EXT. 1.0 means the backend cannot filter anisotropically, @@ -291,6 +306,7 @@ namespace MobileGL { Uint32 SubgroupSupportedStages = 0; Uint32 SubgroupSupportedFeatures = 0; Bool SubgroupQuadOperationsInAllStages = false; + GpuVendorKind GpuVendor = GpuVendorKind::Unknown; }; enum class WindowBackend { diff --git a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp index 5d1ea97e..bac6689b 100644 --- a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp @@ -1035,6 +1035,32 @@ namespace MobileGL::MG_Backend::DirectGLES { m_dynamicParameters.ViewportSubpixelBits = m_GLESCapabilities.ViewportSubpixelBits; m_dynamicParameters.SupportsWideLines = m_GLESCapabilities.AliasedLineWidthRangeMax > 1.0f || m_GLESCapabilities.SmoothLineWidthRangeMax > 1.0f; + + const auto containsAny = [](const String& haystack, std::initializer_list needles) { + return std::any_of(needles.begin(), needles.end(), [&](const char* needle) { + return haystack.find(needle) != String::npos; + }); + }; + const String vendorAndRenderer = + m_GLESCapabilities.GLESVendorString + " " + m_GLESCapabilities.GLESRendererString; + if (containsAny(vendorAndRenderer, {"llvmpipe", "SwiftShader", "softpipe"})) { + // Check software rasterizers first: ANGLE-on-llvmpipe reports both. + m_dynamicParameters.GpuVendor = GpuVendorKind::Software; + } else if (containsAny(vendorAndRenderer, {"Qualcomm", "Adreno"})) { + m_dynamicParameters.GpuVendor = GpuVendorKind::Qualcomm; + } else if (containsAny(vendorAndRenderer, {"Mali", "ARM"})) { + m_dynamicParameters.GpuVendor = GpuVendorKind::Arm; + } else if (containsAny(vendorAndRenderer, {"NVIDIA"})) { + m_dynamicParameters.GpuVendor = GpuVendorKind::Nvidia; + } else if (containsAny(vendorAndRenderer, {"AMD", "Radeon"})) { + m_dynamicParameters.GpuVendor = GpuVendorKind::Amd; + } else if (containsAny(vendorAndRenderer, {"Intel"})) { + m_dynamicParameters.GpuVendor = GpuVendorKind::Intel; + } else if (containsAny(vendorAndRenderer, {"Imagination", "PowerVR"})) { + m_dynamicParameters.GpuVendor = GpuVendorKind::ImgTec; + } else { + m_dynamicParameters.GpuVendor = GpuVendorKind::Unknown; + } } const MG_External::GLESFunctionsTable& BackendObject_DirectGLES::GetGLESFunctions() const { diff --git a/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp index e6a009a5..a10ee89a 100644 --- a/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/BackendObject_DirectVulkan.cpp @@ -794,5 +794,32 @@ namespace MobileGL::MG_Backend::DirectVulkan { m_vulkanCaps.MaxShaderStorageBlockSize, m_dynamicParameters.MaxShaderStorageBlockSize); } + switch (m_vulkanCaps.VendorId) { + case 0x5143u: // VK_VENDOR_ID: Qualcomm + m_dynamicParameters.GpuVendor = GpuVendorKind::Qualcomm; + break; + case 0x13B5u: // ARM + m_dynamicParameters.GpuVendor = GpuVendorKind::Arm; + break; + case 0x10DEu: // NVIDIA + m_dynamicParameters.GpuVendor = GpuVendorKind::Nvidia; + break; + case 0x1002u: // AMD + m_dynamicParameters.GpuVendor = GpuVendorKind::Amd; + break; + case 0x8086u: // Intel + m_dynamicParameters.GpuVendor = GpuVendorKind::Intel; + break; + case 0x1010u: // Imagination + m_dynamicParameters.GpuVendor = GpuVendorKind::ImgTec; + break; + case 0x10005u: // Mesa software (lavapipe) + case 0x1AE0u: // Google (SwiftShader) + m_dynamicParameters.GpuVendor = GpuVendorKind::Software; + break; + default: + m_dynamicParameters.GpuVendor = GpuVendorKind::Unknown; + break; + } } } // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index 6319b262..6114fc24 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp @@ -1623,4 +1623,26 @@ TEST_F(ProgramUtilTest, RewriteLinearSubgroupPrefixScanRejectsPartialOrUnsafeTem ASSERT_NE(consumerEnd, String::npos); nestedScan.insert(consumerEnd + 1, "\n }"); expectUnchanged(std::move(nestedScan)); + + // ARB/NV spellings of lane-width-sensitive builtins must block the rewrite exactly + // like their KHR counterparts. + String arbSubgroupBuiltin = MakeLinearSubgroupPrefixScanShader(); + arbSubgroupBuiltin.insert(arbSubgroupBuiltin.find("float importance"), + "uint arbLane = gl_SubGroupInvocationARB;\n "); + expectUnchanged(std::move(arbSubgroupBuiltin)); + + String arbBallotCall = MakeLinearSubgroupPrefixScanShader(); + arbBallotCall.insert(arbBallotCall.find("float importance"), + "uint64_t arbMask = ballotARB(true);\n "); + expectUnchanged(std::move(arbBallotCall)); + + String nvWarpBuiltin = MakeLinearSubgroupPrefixScanShader(); + nvWarpBuiltin.insert(nvWarpBuiltin.find("float importance"), + "uint warpSize = gl_WarpSizeNV;\n "); + expectUnchanged(std::move(nvWarpBuiltin)); + + String nvShuffleCall = MakeLinearSubgroupPrefixScanShader(); + nvShuffleCall.insert(nvShuffleCall.find("float importance"), + "float other = shuffleNV(1.0f, 0u, 32u);\n "); + expectUnchanged(std::move(nvShuffleCall)); } diff --git a/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp b/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp index a0e5e667..2e710e49 100644 --- a/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp +++ b/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp @@ -121,6 +121,7 @@ namespace MobileGL::MG_Util::BackendLoader { caps.VulkanAPIVersion = DecodeApiVersion(p.apiVersion); caps.DeviceName = p.deviceName; caps.DriverVersionString = DecodeDriverVersion(p.driverVersion); + caps.VendorId = p.vendorID; caps.UniformBufferOffsetAlignment = static_cast(p.limits.minUniformBufferOffsetAlignment); caps.AliasedLineWidthRangeMin = p.limits.lineWidthRange[0]; caps.AliasedLineWidthRangeMax = p.limits.lineWidthRange[1]; @@ -210,6 +211,7 @@ namespace MobileGL::MG_Util::BackendLoader { caps.VulkanAPIVersion = DecodeApiVersion(properties.apiVersion); caps.DeviceName = properties.deviceName; caps.DriverVersionString = DecodeDriverVersion(properties.driverVersion); + caps.VendorId = properties.vendorID; caps.UniformBufferOffsetAlignment = static_cast(properties.limits.minUniformBufferOffsetAlignment); caps.AliasedLineWidthRangeMin = properties.limits.lineWidthRange[0]; caps.AliasedLineWidthRangeMax = properties.limits.lineWidthRange[1]; diff --git a/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.h b/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.h index eec0dcc3..6114579e 100644 --- a/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.h +++ b/MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.h @@ -15,6 +15,8 @@ namespace MobileGL { Version VulkanAPIVersion{1, 0, 0}; String DeviceName; String DriverVersionString; + // VkPhysicalDeviceProperties::vendorID, for device-quirk vendor gating. + Uint32 VendorId = 0; Int UniformBufferOffsetAlignment = 256; Float AliasedLineWidthRangeMin = 1.0f; Float AliasedLineWidthRangeMax = 1.0f; diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp index 4bf6d0dc..ba521a8e 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include namespace { @@ -365,7 +366,21 @@ namespace { HasIdentifierWithPrefixOutsideAllowed(tokens, "subgroup", {"subgroupInclusiveAdd"}) || HasIdentifierWithPrefixOutsideAllowed( tokens, "gl_Subgroup", - {"gl_SubgroupInvocationID", "gl_SubgroupSize", "gl_SubgroupID", "gl_NumSubgroups"})) { + {"gl_SubgroupInvocationID", "gl_SubgroupSize", "gl_SubgroupID", "gl_NumSubgroups"}) || + // ARB/NV spellings of lane-width-sensitive builtins and functions + // (gl_SubGroupSizeARB, ballotARB, gl_WarpSizeNV, shuffleNV, ...) must block the + // rewrite just like their KHR counterparts: they would silently keep native-width + // semantics in a module rewritten to the virtual 32-lane model. + HasIdentifierWithPrefixOutsideAllowed(tokens, "gl_SubGroup", {}) || + HasIdentifierWithPrefixOutsideAllowed(tokens, "gl_Warp", {}) || + HasIdentifierWithPrefixOutsideAllowed(tokens, "gl_Thread", {}) || + HasIdentifierWithPrefixOutsideAllowed(tokens, "gl_SMID", {}) || + HasIdentifierWithPrefixOutsideAllowed(tokens, "ballot", {}) || + HasIdentifierWithPrefixOutsideAllowed(tokens, "shuffle", {}) || + HasIdentifierWithPrefixOutsideAllowed(tokens, "readInvocation", {}) || + HasIdentifierWithPrefixOutsideAllowed(tokens, "readFirstInvocation", {}) || + HasIdentifierWithPrefixOutsideAllowed(tokens, "anyInvocation", {}) || + HasIdentifierWithPrefixOutsideAllowed(tokens, "allInvocations", {})) { return false; } @@ -967,6 +982,14 @@ namespace MobileGL { const Vector tokens = TokenizeCode(source); LinearPrefixScanMatch match; if (!ParseLinearPrefixScanTemplate(tokens, match)) { + // Diagnosability: when the trigger op is present but the template no longer + // matches (e.g. the pack shipped a new shader revision), the affected device + // silently falls back to the driver's miscompiled path. Make that visible. + if (CountToken(tokens, "subgroupInclusiveAdd") > 0) { + MGLOG_W("%s: subgroupInclusiveAdd present but the linear prefix-scan template " + "did not match; the wide-subgroup rewrite was NOT applied", + __func__); + } return false; } @@ -979,6 +1002,76 @@ namespace MobileGL { return true; } + namespace { + struct ShaderSourceQuirkContext { + ShaderStage stage = ShaderStage::Unknown; + BackendType backend = BackendType::Unknown; + MG_Backend::GpuVendorKind vendor = MG_Backend::GpuVendorKind::Unknown; + Uint32 subgroupSize = 0; + }; + + // Device-quirk registry. Every entry is a narrowly scoped source rewrite that + // works around a specific driver defect. A quirk runs when its env override + // forces it on, or when the override is Auto and DeviceApplies matches the + // detected device. ForceOn bypasses only the device gate - each Apply keeps + // its own structural safety checks. Add new per-device workarounds here + // instead of open-coding them in PreprocessShaderSource. + struct ShaderSourceQuirk { + const char* name; + MG_Config::QuirkOverride (*GetOverride)(); + Bool (*DeviceApplies)(const ShaderSourceQuirkContext&); + Bool (*Apply)(const ShaderSourceQuirkContext&, String&); + }; + + constexpr ShaderSourceQuirk kShaderSourceQuirks[] = { + { + // MOBILEGL_QUIRK_SUBGROUP_PREFIX_SCAN + "subgroup-prefix-scan-rewrite", + [] { return MG_Config::Features.SubgroupPrefixScanQuirk; }, + [](const ShaderSourceQuirkContext& ctx) { + // Qualcomm's Vulkan driver miscompiles the recognized float + // InclusiveScan pattern for native subgroups wider than the + // captured 32 lanes; other vendors compile it correctly and + // should keep their native scan. + return ctx.backend == BackendType::DirectVulkan && + ctx.vendor == MG_Backend::GpuVendorKind::Qualcomm; + }, + [](const ShaderSourceQuirkContext& ctx, String& source) { + return RewriteLinearSubgroupPrefixScanForVulkan(ctx.stage, ctx.subgroupSize, + source); + }, + }, + }; + + void ApplyShaderSourceQuirks(ShaderStage stage, String& source) { + const auto& activeBackend = MG_Backend::pActiveBackendObject; + if (!activeBackend) { + return; + } + const auto& dynamicParameters = activeBackend->GetDynamicParameters(); + const ShaderSourceQuirkContext quirkContext{ + stage, + activeBackend->GetBackendType(), + dynamicParameters.GpuVendor, + dynamicParameters.SubgroupSize, + }; + for (const ShaderSourceQuirk& quirk : kShaderSourceQuirks) { + const MG_Config::QuirkOverride quirkOverride = quirk.GetOverride(); + if (quirkOverride == MG_Config::QuirkOverride::ForceOff) { + continue; + } + if (quirkOverride == MG_Config::QuirkOverride::Auto && + !quirk.DeviceApplies(quirkContext)) { + continue; + } + if (quirk.Apply(quirkContext, source)) { + MGLOG_I("ApplyShaderSourceQuirks: applied '%s'%s", quirk.name, + quirkOverride == MG_Config::QuirkOverride::ForceOn ? " (forced on)" : ""); + } + } + } + } // namespace + void PreprocessShaderSource(ShaderStage stage, String& source) { // Normalize while the inspector's source span still refers to the untouched input. Later passes // remove comments and directives, so any subsequent insertion re-inspects the current source. @@ -1035,12 +1128,7 @@ namespace MobileGL { ModernizeLegacyGLSL(stage, source); InjectDepthRangeBuiltinShim(stage, source); - const auto& activeBackend = MG_Backend::pActiveBackendObject; - if (stage == ShaderStage::Compute && activeBackend && - activeBackend->GetBackendType() == BackendType::DirectVulkan) { - RewriteLinearSubgroupPrefixScanForVulkan(stage, activeBackend->GetDynamicParameters().SubgroupSize, - source); - } + ApplyShaderSourceQuirks(stage, source); } Bool RetargetLegacyVersionDirectiveTo460(String& source) { diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.h b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.h index e6c318ee..923448a5 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.h +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.h @@ -27,8 +27,10 @@ namespace MobileGL { // wider than the capture's 32 lanes. For the narrowly recognized, uniform-control- // flow template, replace the subgroup-local scan with a shared-memory, strict // left-fold over virtual 32-lane segments. Returns true only when the complete safe - // template was recognized and rewritten. DirectVulkan calls this through - // PreprocessShaderSource; the explicit entry point exists for deterministic tests. + // template was recognized and rewritten. PreprocessShaderSource reaches this through + // its device-quirk registry: by default only on detected Qualcomm Vulkan devices, + // overridable either way with MOBILEGL_QUIRK_SUBGROUP_PREFIX_SCAN=1/0. The explicit + // entry point exists for deterministic tests. Bool RewriteLinearSubgroupPrefixScanForVulkan(ShaderStage stage, Uint32 nativeSubgroupSize, String& source); // Rewrites a "#version 330 core" directive that PreprocessShaderSource normalized down