Reland "Remove explicitlyCastMediumpFloatTo16Bit workaround feature" Reland commit ad00a98e67db0038929c52199c040da2766c2dfb. The prerequisite test fix https://gerrit.khronos.org/c/vk-gl-cts/+/19028 has landed in Android. Original change's description: > Remove explicitlyCastMediumpFloatTo16Bit workaround feature > > The test bug that led to this workaround is fixed in > https://gerrit.khronos.org/c/vk-gl-cts/+/19028. We no longer need this > workaround feature. > > Test ShaderAlgorithmTest.rgb_to_hsl_vertex_shader still fails on Windows > intel gpu even changing the precision of a_unitCoords. Keep the test > failure in the expectation file. > > Bug: b/274859104 > Bug: b/349994211 > Change-Id: I0d90e1e381c2781ae374985bef8dd92f5e400514 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7530691 > Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> > Commit-Queue: Yuxin Hu <yuxinhu@google.com> Bug: b/274859104 Bug: b/349994211 Change-Id: If6aece987806f8fb9cb441fc55532a7862b04fb4 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7706871 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Yuxin Hu <yuxinhu@google.com>
diff --git a/include/GLSLANG/ShaderLang.h b/include/GLSLANG/ShaderLang.h index bc69ff4..65ad207 100644 --- a/include/GLSLANG/ShaderLang.h +++ b/include/GLSLANG/ShaderLang.h
@@ -26,7 +26,7 @@ // Version number for shader translation API. // It is incremented every time the API changes. -#define ANGLE_SH_VERSION 406 +#define ANGLE_SH_VERSION 407 enum ShShaderSpec { @@ -410,13 +410,10 @@ // ceil()ed instead. uint64_t roundOutputAfterDithering : 1; - // issuetracker.google.com/274859104 add OpQuantizeToF16 instruction to cast - // mediump floating-point values to 16 bit. ARM compiler utilized RelaxedPrecision - // to minimize type case and keep a mediump float as 32 bit when assigning it with - // a highp floating-point value. It is possible that GLSL shader code is comparing - // two meiump values, but ARM compiler is comparing a 32 bit value with a 16 bit value, - // causing the comparison to fail. - uint64_t castMediumpFloatTo16Bit : 1; + // placeholder bit for removed castMediumpFloatTo16Bit option. This is needed because chromium + // fuzzer needs the ShCompileOptions memory layout remains unchanged to be able to map the bugs + // filed. + uint64_t unused3 : 1; // anglebug.com/42265995: packUnorm4x8 fails on Pixel 4 if it is not passed a highp vec4. // TODO(anglebug.com/42265995): This workaround is currently only applied for pixel local
diff --git a/include/platform/autogen/FeaturesVk_autogen.h b/include/platform/autogen/FeaturesVk_autogen.h index 81b892d..562cb0d 100644 --- a/include/platform/autogen/FeaturesVk_autogen.h +++ b/include/platform/autogen/FeaturesVk_autogen.h
@@ -854,12 +854,6 @@ &members, }; - FeatureInfo explicitlyCastMediumpFloatTo16Bit = { - "explicitlyCastMediumpFloatTo16Bit", - FeatureCategory::VulkanWorkarounds, - &members, - }; - FeatureInfo forceContinuousRefreshOnSharedPresent = { "forceContinuousRefreshOnSharedPresent", FeatureCategory::VulkanFeatures,
diff --git a/include/platform/vk_features.json b/include/platform/vk_features.json index 739cb26..6b78aea 100644 --- a/include/platform/vk_features.json +++ b/include/platform/vk_features.json
@@ -1109,14 +1109,6 @@ "issue": "http://anglebug.com/42265369" }, { - "name": "explicitly_cast_mediump_float_to_16_bit", - "category": "Workarounds", - "description": [ - "Explicitly cast mediump floating point values to 16 bit" - ], - "issue": "https://issuetracker.google.com/274859104" - }, - { "name": "force_continuous_refresh_on_shared_present", "category": "Features", "description": [
diff --git a/src/compiler/translator/spirv/OutputSPIRV.cpp b/src/compiler/translator/spirv/OutputSPIRV.cpp index 8836a5e..9b161cd 100644 --- a/src/compiler/translator/spirv/OutputSPIRV.cpp +++ b/src/compiler/translator/spirv/OutputSPIRV.cpp
@@ -6201,7 +6201,6 @@ TIntermSymbol *symbol = sequence.front()->getAsSymbolNode(); spirv::IdRef initializerId; bool initializeWithDeclaration = false; - bool needsQuantizeTo16 = false; // Handle declarations with initializer. if (symbol == nullptr) @@ -6250,35 +6249,6 @@ { // Otherwise generate code to load from right hand side expression. initializerId = accessChainLoad(&mNodeData.back(), symbol->getType(), nullptr); - - // Workaround for issuetracker.google.com/274859104 - // ARM SpirV compiler may utilize the RelaxedPrecision of mediump float, - // and chooses to not cast mediump float to 16 bit. This causes deqp test - // dEQP-GLES2.functional.shaders.algorithm.rgb_to_hsl_vertex failed. - // The reason is that GLSL shader code expects below condition to be true: - // mediump float a == mediump float b; - // However, the condition is false after translating to SpirV - // due to one of them is 32 bit, and the other is 16 bit. - // To resolve the deqp test failure, we will add an OpQuantizeToF16 - // SpirV instruction to explicitly cast mediump float scalar or mediump float - // vector to 16 bit, if the right-hand-side is a highp float. - if (mCompileOptions.castMediumpFloatTo16Bit) - { - const TType leftType = assign->getLeft()->getType(); - const TType rightType = assign->getRight()->getType(); - const TPrecision leftPrecision = leftType.getPrecision(); - const TPrecision rightPrecision = rightType.getPrecision(); - const bool isLeftScalarFloat = leftType.isScalarFloat(); - const bool isLeftVectorFloat = leftType.isVector() && !leftType.isVectorArray() && - leftType.getBasicType() == EbtFloat; - - if (leftPrecision == TPrecision::EbpMedium && - rightPrecision == TPrecision::EbpHigh && - (isLeftScalarFloat || isLeftVectorFloat)) - { - needsQuantizeTo16 = true; - } - } } // Clean up the initializer data. @@ -6337,17 +6307,6 @@ if (!initializeWithDeclaration && initializerId.valid()) { - // If not initializing at the same time as the declaration, issue a store - if (needsQuantizeTo16) - { - // Insert OpQuantizeToF16 instruction to explicitly cast mediump float to 16 bit before - // issuing an OpStore instruction. - const spirv::IdRef quantizeToF16Result = - mBuilder.getNewId(mBuilder.getDecorations(symbol->getType())); - spirv::WriteQuantizeToF16(mBuilder.getSpirvCurrentFunctionBlock(), typeId, - quantizeToF16Result, initializerId); - initializerId = quantizeToF16Result; - } spirv::WriteStore(mBuilder.getSpirvCurrentFunctionBlock(), variableId, initializerId, nullptr); }
diff --git a/src/libANGLE/renderer/vulkan/ShaderVk.cpp b/src/libANGLE/renderer/vulkan/ShaderVk.cpp index 661f23b..d5e0e29 100644 --- a/src/libANGLE/renderer/vulkan/ShaderVk.cpp +++ b/src/libANGLE/renderer/vulkan/ShaderVk.cpp
@@ -110,11 +110,6 @@ options->aliasedUnlessRestrict = true; } - if (contextVk->getFeatures().explicitlyCastMediumpFloatTo16Bit.enabled) - { - options->castMediumpFloatTo16Bit = true; - } - if (contextVk->getExtensions().shaderPixelLocalStorageANGLE) { options->pls = contextVk->getNativePixelLocalStorageOptions();
diff --git a/src/libANGLE/renderer/vulkan/vk_renderer.cpp b/src/libANGLE/renderer/vulkan/vk_renderer.cpp index f9ce672..ff63925 100644 --- a/src/libANGLE/renderer/vulkan/vk_renderer.cpp +++ b/src/libANGLE/renderer/vulkan/vk_renderer.cpp
@@ -6485,8 +6485,6 @@ // enabling per-sample shading pipeline state. ANGLE_FEATURE_CONDITION(&mFeatures, explicitlyEnablePerSampleShading, !isQualcommProprietary); - ANGLE_FEATURE_CONDITION(&mFeatures, explicitlyCastMediumpFloatTo16Bit, isARMProprietary); - // Force to create swapchain with continuous refresh on shared present. Disabled by default. // Only enable it on integrations without EGL_FRONT_BUFFER_AUTO_REFRESH_ANDROID passthrough. ANGLE_FEATURE_CONDITION(&mFeatures, forceContinuousRefreshOnSharedPresent, false);
diff --git a/src/tests/deqp_support/deqp_gles2_test_expectations.txt b/src/tests/deqp_support/deqp_gles2_test_expectations.txt index 29648f9..65e2f5b 100644 --- a/src/tests/deqp_support/deqp_gles2_test_expectations.txt +++ b/src/tests/deqp_support/deqp_gles2_test_expectations.txt
@@ -512,7 +512,3 @@ 356399840 WGPU : dEQP-GLES2.functional.uniform_api.value.*.render.* = FAIL 356399840 WGPU : dEQP-GLES2.functional.uniform_api.random.* = FAIL 392542001 WGPU : dEQP-GLES2.functional.vertex_arrays.multiple_attributes* = FAIL - -// Translator IR failures -// The workaround in b/274859104 is not applied because the IR does not generate non-constant initializers -274859104 IR PIXEL6 VULKAN : dEQP-GLES2.functional.shaders.algorithm.rgb_to_hsl_vertex = SKIP
diff --git a/src/tests/gl_tests/ShaderAlgorithmTest.cpp b/src/tests/gl_tests/ShaderAlgorithmTest.cpp index 5956143..141eb37 100644 --- a/src/tests/gl_tests/ShaderAlgorithmTest.cpp +++ b/src/tests/gl_tests/ShaderAlgorithmTest.cpp
@@ -28,7 +28,7 @@ TEST_P(ShaderAlgorithmTest, rgb_to_hsl_vertex_shader) { const char kVS[] = R"(attribute highp vec3 a_position; -attribute highp vec3 a_unitCoords; +attribute mediump vec3 a_unitCoords; varying mediump vec3 v_color; void main()
diff --git a/util/autogen/angle_features_autogen.cpp b/util/autogen/angle_features_autogen.cpp index 5485ec3..f072829 100644 --- a/util/autogen/angle_features_autogen.cpp +++ b/util/autogen/angle_features_autogen.cpp
@@ -171,7 +171,6 @@ {Feature::EnsureNonEmptyBufferIsBoundForDraw, "ensureNonEmptyBufferIsBoundForDraw"}, {Feature::ExpandIntegerPowExpressions, "expandIntegerPowExpressions"}, {Feature::ExplicitFragmentLocations, "explicitFragmentLocations"}, - {Feature::ExplicitlyCastMediumpFloatTo16Bit, "explicitlyCastMediumpFloatTo16Bit"}, {Feature::ExplicitlyEnablePerSampleShading, "explicitlyEnablePerSampleShading"}, {Feature::ExposeES32ForTesting, "exposeES32ForTesting"}, {Feature::ExposeNonConformantExtensionsAndVersions, "exposeNonConformantExtensionsAndVersions"},
diff --git a/util/autogen/angle_features_autogen.h b/util/autogen/angle_features_autogen.h index fdc51c8..2edb588 100644 --- a/util/autogen/angle_features_autogen.h +++ b/util/autogen/angle_features_autogen.h
@@ -171,7 +171,6 @@ EnsureNonEmptyBufferIsBoundForDraw, ExpandIntegerPowExpressions, ExplicitFragmentLocations, - ExplicitlyCastMediumpFloatTo16Bit, ExplicitlyEnablePerSampleShading, ExposeES32ForTesting, ExposeNonConformantExtensionsAndVersions,