GLSL: Fix expression reorder bug with legacy FMA fallback.

This commit is contained in:
Hans-Kristian Arntzen 2023-03-30 16:51:05 +02:00
parent 8e64f8ee40
commit 50623e13c8
7 changed files with 53 additions and 3 deletions

View File

@ -8,6 +8,8 @@ varying highp vec4 vC;
void main() void main()
{ {
gl_FragData[0] = vA * vB + vC; highp vec4 _17 = vA * vB + vC;
gl_FragData[0] = _17;
gl_FragData[0] = _17 * (vB * vC + vA);
} }

View File

@ -0,0 +1,10 @@
#version 310 es
layout(location = 0) out float a;
void main()
{
a = 5.0;
a = a * 2.0 + 1.0;
}

View File

@ -9,5 +9,6 @@ varying highp vec4 vC;
void main() void main()
{ {
gl_FragData[0] = vA * vB + vC; gl_FragData[0] = vA * vB + vC;
gl_FragData[0] = (vA * vB + vC) * (vB * vC + vA);
} }

View File

@ -0,0 +1,24 @@
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Vertex %main "main" %a
OpSource ESSL 310
OpName %main "main"
OpName %a "a"
OpDecorate %a Location 0
%void = OpTypeVoid
%3 = OpTypeFunction %void
%float = OpTypeFloat 32
%_ptr_Output_float = OpTypePointer Output %float
%a = OpVariable %_ptr_Output_float Output
%float_5 = OpConstant %float 5
%float_2 = OpConstant %float 2
%float_1 = OpConstant %float 1
%main = OpFunction %void None %3
%5 = OpLabel
OpStore %a %float_5
%10 = OpLoad %float %a
%14 = OpExtInst %float %1 Fma %10 %float_2 %float_1
OpStore %a %14
OpReturn
OpFunctionEnd

View File

@ -8,4 +8,5 @@ layout(location = 0) out vec4 FragColor;
void main() void main()
{ {
FragColor = fma(vA, vB, vC); FragColor = fma(vA, vB, vC);
FragColor = fma(vA, vB, vC) * fma(vB, vC, vA);
} }

View File

@ -4688,7 +4688,7 @@ void CompilerGLSL::strip_enclosed_expression(string &expr)
expr.erase(begin(expr)); expr.erase(begin(expr));
} }
string CompilerGLSL::enclose_expression(const string &expr) bool CompilerGLSL::needs_enclose_expression(const std::string &expr)
{ {
bool need_parens = false; bool need_parens = false;
@ -4722,10 +4722,15 @@ string CompilerGLSL::enclose_expression(const string &expr)
assert(paren_count == 0); assert(paren_count == 0);
} }
return need_parens;
}
string CompilerGLSL::enclose_expression(const string &expr)
{
// If this expression contains any spaces which are not enclosed by parentheses, // If this expression contains any spaces which are not enclosed by parentheses,
// we need to enclose it so we can treat the whole string as an expression. // we need to enclose it so we can treat the whole string as an expression.
// This happens when two expressions have been part of a binary op earlier. // This happens when two expressions have been part of a binary op earlier.
if (need_parens) if (needs_enclose_expression(expr))
return join('(', expr, ')'); return join('(', expr, ')');
else else
return expr; return expr;
@ -10897,6 +10902,12 @@ bool CompilerGLSL::optimize_read_modify_write(const SPIRType &type, const string
char bop = rhs[op]; char bop = rhs[op];
auto expr = rhs.substr(lhs.size() + 3); auto expr = rhs.substr(lhs.size() + 3);
// Avoids false positives where we get a = a * b + c.
// Normally, these expressions are always enclosed, but unexpected code paths may end up hitting this.
if (needs_enclose_expression(expr))
return false;
// Try to find increments and decrements. Makes it look neater as += 1, -= 1 is fairly rare to see in real code. // Try to find increments and decrements. Makes it look neater as += 1, -= 1 is fairly rare to see in real code.
// Find some common patterns which are equivalent. // Find some common patterns which are equivalent.
if ((bop == '+' || bop == '-') && (expr == "1" || expr == "uint(1)" || expr == "1u" || expr == "int(1u)")) if ((bop == '+' || bop == '-') && (expr == "1" || expr == "uint(1)" || expr == "1u" || expr == "int(1u)"))

View File

@ -769,6 +769,7 @@ protected:
std::string to_extract_component_expression(uint32_t id, uint32_t index); std::string to_extract_component_expression(uint32_t id, uint32_t index);
std::string to_extract_constant_composite_expression(uint32_t result_type, const SPIRConstant &c, std::string to_extract_constant_composite_expression(uint32_t result_type, const SPIRConstant &c,
const uint32_t *chain, uint32_t length); const uint32_t *chain, uint32_t length);
static bool needs_enclose_expression(const std::string &expr);
std::string enclose_expression(const std::string &expr); std::string enclose_expression(const std::string &expr);
std::string dereference_expression(const SPIRType &expression_type, const std::string &expr); std::string dereference_expression(const SPIRType &expression_type, const std::string &expr);
std::string address_of_expression(const std::string &expr); std::string address_of_expression(const std::string &expr);