Commit bb27c3a1 by Olli Etuaho Committed by Commit Bot

Fix VectorizeVectorScalarArithmetic statement insertion

The traverser must avoid inserting two statements to the same position on a single traversal, so it doesn't trigger an assert. BUG=chromium:784078 TEST=angle_unittests Change-Id: I855054e62cc1b1cf4e6bb02af527954151c7d0e7 Reviewed-on: https://chromium-review.googlesource.com/771611 Commit-Queue: Olli Etuaho <oetuaho@nvidia.com> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org>
parent 1eda27a6
......@@ -122,6 +122,15 @@ TIntermTraverser::~TIntermTraverser()
{
}
const TIntermBlock *TIntermTraverser::getParentBlock() const
{
if (!mParentBlockStack.empty())
{
return mParentBlockStack.back().node;
}
return nullptr;
}
void TIntermTraverser::pushParentBlock(TIntermBlock *node)
{
mParentBlockStack.push_back(ParentBlock(node, 0));
......
......@@ -143,6 +143,8 @@ class TIntermTraverser : angle::NonCopyable
return nullptr;
}
const TIntermBlock *getParentBlock() const;
void pushParentBlock(TIntermBlock *node);
void incrementParentBlockPos();
void popParentBlock();
......
......@@ -11,6 +11,8 @@
#include "compiler/translator/VectorizeVectorScalarArithmetic.h"
#include <set>
#include "compiler/translator/IntermNode.h"
#include "compiler/translator/IntermTraverse.h"
......@@ -29,7 +31,11 @@ class VectorizeVectorScalarArithmeticTraverser : public TIntermTraverser
}
bool didReplaceScalarsWithVectors() { return mReplaced; }
void nextIteration() { mReplaced = false; }
void nextIteration()
{
mReplaced = false;
mModifiedBlocks.clear();
}
protected:
bool visitBinary(Visit visit, TIntermBinary *node) override;
......@@ -47,6 +53,7 @@ class VectorizeVectorScalarArithmeticTraverser : public TIntermTraverser
TIntermTraverser::OriginalNode *originalNodeFate);
bool mReplaced;
std::set<const TIntermBlock *> mModifiedBlocks;
};
TIntermTyped *VectorizeVectorScalarArithmeticTraverser::Vectorize(
......@@ -241,11 +248,17 @@ bool VectorizeVectorScalarArithmeticTraverser::visitAggregate(Visit /*visit*/,
// leave that be.
if (!argBinary->getLeft()->hasSideEffects())
{
replaceAssignInsideConstructor(node, argBinary);
mReplaced = true;
// Don't replace more nodes in the same subtree on this traversal.
// However, nodes elsewhere in the tree may still be replaced.
return false;
const TIntermBlock *parentBlock = getParentBlock();
// We can't do more than one insertion to the same block on the same traversal.
if (mModifiedBlocks.find(parentBlock) == mModifiedBlocks.end())
{
replaceAssignInsideConstructor(node, argBinary);
mModifiedBlocks.insert(parentBlock);
mReplaced = true;
// Don't replace more nodes in the same subtree on this traversal.
// However, nodes elsewhere in the tree may still be replaced.
return false;
}
}
break;
}
......
......@@ -88,6 +88,7 @@
'<(angle_path)/src/tests/compiler_tests/ShCompile_test.cpp',
'<(angle_path)/src/tests/compiler_tests/TypeTracking_test.cpp',
'<(angle_path)/src/tests/compiler_tests/VariablePacker_test.cpp',
'<(angle_path)/src/tests/compiler_tests/VectorizeVectorScalarArithmetic_test.cpp',
'<(angle_path)/src/tests/compiler_tests/WEBGL_multiview_test.cpp',
'<(angle_path)/src/tests/compiler_tests/WorkGroupSize_test.cpp',
'<(angle_path)/src/tests/preprocessor_tests/char_test.cpp',
......
//
// Copyright (c) 2017 The ANGLE Project Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// VectorizeVectorScalarArithmetic_test.cpp:
// Tests shader compilation with SH_REWRITE_VECTOR_SCALAR_ARITHMETIC workaround on.
#include "GLSLANG/ShaderLang.h"
#include "angle_gl.h"
#include "gtest/gtest.h"
#include "tests/test_utils/ShaderCompileTreeTest.h"
using namespace sh;
class VectorizeVectorScalarArithmeticTest : public ShaderCompileTreeTest
{
public:
VectorizeVectorScalarArithmeticTest() : ShaderCompileTreeTest()
{
mExtraCompileOptions = SH_REWRITE_VECTOR_SCALAR_ARITHMETIC;
}
protected:
::GLenum getShaderType() const override { return GL_FRAGMENT_SHADER; }
ShShaderSpec getShaderSpec() const override { return SH_GLES3_1_SPEC; }
};
// Test that two ops that generate statements in the parent block inside the same statement don't
// trigger an assert.
TEST_F(VectorizeVectorScalarArithmeticTest, TwoMutatedOpsWithSideEffectsInsideSameStatement)
{
const std::string &shaderString =
R"(#version 300 es
precision highp float;
out vec4 res;
uniform float uf;
void main()
{
res = vec4(0.0);
float f = uf;
res += f *= f, res += f *= f;
})";
if (!compile(shaderString))
{
FAIL() << "Shader compilation failed, expecting success:\n" << mInfoLog;
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment