commit 9112b1a96d49967d87142179758ff6f32ca132e8
author Shahbaz Youssefi <syoussefi@chromium.org> Mon Sep 13 19:39:53 2021
committer Angle LUCI CQ <angle-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Sep 14 15:56:36 2021
tree 580d5c70ea5b483292d3456d8e2f1442fb9162f7
parent f2b7de84ba84a48d638f96f02f217f3261f5523f

Vulkan: SPIR-V Gen: Fix precision of findMsb

In GLSL, findMsb operates on a highp operand, while returning a lowp
result.  In SPIR-V, RelaxedPrecision cannot be applied on the FindSMsb
instruction as that affects the operand as well.  This change makes sure
RelaxedPrecision is not applied to a FindSMsb instruction.

Bug: angleproject:4889
Bug: angleproject:6132
Change-Id: I6a0defbbd53ec703c7ecbad5cd5c79d215b11c32
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3158506
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

src/compiler/translator/BuildSPIRV.cpp

diff --git a/src/compiler/translator/BuildSPIRV.cpp b/src/compiler/translator/BuildSPIRV.cpp
index aa52769..a80852e 100644
--- a/src/compiler/translator/BuildSPIRV.cpp
+++ b/src/compiler/translator/BuildSPIRV.cpp
@@ -675,10 +675,37 @@
     return decorations;
 }
 
-SpirvDecorations SPIRVBuilder::getArithmeticDecorations(const TType &type, bool isPrecise)
+SpirvDecorations SPIRVBuilder::getArithmeticDecorations(const TType &type,
+                                                        bool isPrecise,
+                                                        TOperator op)
 {
     SpirvDecorations decorations = getDecorations(type);
 
+    // In GLSL, findMsb operates on a highp operand, while returning a lowp result.  In SPIR-V,
+    // RelaxedPrecision cannot be applied on the Find*Msb instructions as that affects the operand
+    // as well:
+    //
+    // > The RelaxedPrecision Decoration can be applied to:
+    // > ...
+    // > The Result <id> of an instruction that operates on numerical types, meaning the instruction
+    // > is to operate at relaxed precision. The instruction's operands may also be truncated to the
+    // > relaxed precision.
+    // > ...
+    //
+    // Here, we remove RelaxedPrecision from such problematic instructions.
+    switch (op)
+    {
+        case EOpFindMSB:
+            // Currently getDecorations() only adds RelaxedPrecision, so removing the
+            // RelaxedPrecision decoration is simply done by clearing the vector.
+            ASSERT(decorations.empty() ||
+                   (decorations.size() == 1 && decorations[0] == spv::DecorationRelaxedPrecision));
+            decorations.clear();
+            break;
+        default:
+            break;
+    }
+
     // Handle |precise|.
     if (isPrecise)
     {

src/compiler/translator/BuildSPIRV.h

diff --git a/src/compiler/translator/BuildSPIRV.h b/src/compiler/translator/BuildSPIRV.h
index c3be402..4b30401 100644
--- a/src/compiler/translator/BuildSPIRV.h
+++ b/src/compiler/translator/BuildSPIRV.h
@@ -316,7 +316,7 @@
     // Decorations that may apply to intermediate instructions (in addition to variables).
     // |precise| is only applicable to arithmetic nodes.
     SpirvDecorations getDecorations(const TType &type);
-    SpirvDecorations getArithmeticDecorations(const TType &type, bool isPrecise);
+    SpirvDecorations getArithmeticDecorations(const TType &type, bool isPrecise, TOperator op);
 
     // Extended instructions
     spirv::IdRef getExtInstImportIdStd();

src/compiler/translator/OutputSPIRV.cpp

diff --git a/src/compiler/translator/OutputSPIRV.cpp b/src/compiler/translator/OutputSPIRV.cpp
index c6a3233..ce89b7a 100644
--- a/src/compiler/translator/OutputSPIRV.cpp
+++ b/src/compiler/translator/OutputSPIRV.cpp
@@ -2851,7 +2851,7 @@
     }
 
     const SpirvDecorations decorations =
-        mBuilder.getArithmeticDecorations(node->getType(), node->isPrecise());
+        mBuilder.getArithmeticDecorations(node->getType(), node->isPrecise(), op);
     spirv::IdRef result;
     if (node->getType().getBasicType() != EbtVoid)
     {
@@ -2959,7 +2959,7 @@
         {
             const spirv::IdRef one           = mBuilder.getFloatConstant(1);
             const spirv::IdRef invertedParam = mBuilder.getNewId(
-                mBuilder.getArithmeticDecorations(secondChild->getType(), node->isPrecise()));
+                mBuilder.getArithmeticDecorations(secondChild->getType(), node->isPrecise(), op));
             spirv::WriteFDiv(mBuilder.getSpirvCurrentFunctionBlock(), parameterTypeIds.back(),
                              invertedParam, one, parameters[1]);
             parameters[1] = invertedParam;