Commit 7535b761 by Olli Etuaho Committed by Zhenyao Mo

Remove dynamic indexing of matrices and vectors in HLSL

Re-re-landing with fix to setting qualifiers on generated nodes. The previous version failed when a uniform was indexed, because it would set the uniform qualifier on some of the generated nodes and that interfered with the operation of UniformsHLSL. Re-landing after fixing D3D9 specific issues. HLSL doesn't support dynamic indexing of matrices and vectors, so replace that with helper functions that unroll dynamic indexing into switch/case and static indexing. Both the indexed vector/matrix expression and the index may have side effects, and these will be evaluated correctly. If necessary, index expressions that have side effects will be written to a temporary variable that will replace the index. Besides dEQP tests, this change is tested by a WebGL 2 conformance test. In the case that a dynamic index is out-of-range, the base ESSL 3.00 spec allows undefined behavior. KHR_robust_buffer_access_behavior adds the requirement that program termination should not occur and that out-of-range reads must return either a value from the active program's memory or zero, and out-of-range writes should only affect the active program's memory or do nothing. This patch clamps out-of-range indices so that either the first or last item of the matrix/vector is accessed. The code is not transformed in case the it fits within the limited subset of ESSL 1.00 given in Appendix A of the spec. If the code isn't within the restricted subset, even ESSL 1.00 shaders may require this workaround. BUG=angleproject:1116 TEST=dEQP-GLES3.functional.shaders.indexing.* (all pass after change) WebGL 2 conformance tests (glsl3/vector-dynamic-indexing.html) Change-Id: I16119f9092360fb72798f9550a6f4d3cfffdc92f Reviewed-on: https://chromium-review.googlesource.com/308790Reviewed-by: 's avatarZhenyao Mo <zmo@chromium.org> Tested-by: 's avatarZhenyao Mo <zmo@chromium.org>
parent a33475b3
......@@ -159,6 +159,8 @@
'compiler/translator/BuiltInFunctionEmulatorHLSL.h',
'compiler/translator/OutputHLSL.cpp',
'compiler/translator/OutputHLSL.h',
'compiler/translator/RemoveDynamicIndexing.cpp',
'compiler/translator/RemoveDynamicIndexing.h',
'compiler/translator/RemoveSwitchFallThrough.cpp',
'compiler/translator/RemoveSwitchFallThrough.h',
'compiler/translator/RewriteElseBlocks.cpp',
......
......@@ -153,6 +153,15 @@ TCompiler::~TCompiler()
{
}
bool TCompiler::shouldRunLoopAndIndexingValidation(int compileOptions) const
{
// If compiling an ESSL 1.00 shader for WebGL, or if its been requested through the API,
// validate loop and indexing as well (to verify that the shader only uses minimal functionality
// of ESSL 1.00 as in Appendix A of the spec).
return (IsWebGLBasedSpec(shaderSpec) && shaderVersion == 100) ||
(compileOptions & SH_VALIDATE_LOOP_INDEXING);
}
bool TCompiler::Init(const ShBuiltInResources& resources)
{
shaderVersion = 100;
......@@ -230,12 +239,6 @@ TIntermNode *TCompiler::compileTreeImpl(const char *const shaderStrings[],
success = false;
}
// If compiling an ESSL 1.00 shader for WebGL, or if its been requested through the API,
// validate loop and indexing as well (to verify that the shader only uses minimal functionality
// of ESSL 1.00 as in Appendix A of the spec).
bool validateLoopAndIndexing = (IsWebGLBasedSpec(shaderSpec) && shaderVersion == 100) ||
(compileOptions & SH_VALIDATE_LOOP_INDEXING);
TIntermNode *root = nullptr;
if (success)
......@@ -281,7 +284,7 @@ TIntermNode *TCompiler::compileTreeImpl(const char *const shaderStrings[],
if (success && shaderVersion == 300 && shaderType == GL_FRAGMENT_SHADER)
success = validateOutputs(root);
if (success && validateLoopAndIndexing)
if (success && shouldRunLoopAndIndexingValidation(compileOptions))
success = validateLimitations(root);
if (success && (compileOptions & SH_TIMING_RESTRICTIONS))
......
......@@ -101,6 +101,8 @@ class TCompiler : public TShHandleBase
ShShaderOutput getOutputType() const { return outputType; }
const std::string &getBuiltInResourcesString() const { return builtInResourcesString; }
bool shouldRunLoopAndIndexingValidation(int compileOptions) const;
// Get the resources set by InitBuiltInSymbolTable
const ShBuiltInResources& getResources() const;
......
......@@ -311,17 +311,13 @@ bool TIntermAggregate::replaceChildNodeWithMultiple(TIntermNode *original, TInte
bool TIntermAggregate::insertChildNodes(TIntermSequence::size_type position, TIntermSequence insertions)
{
TIntermSequence::size_type itPosition = 0;
for (auto it = mSequence.begin(); it < mSequence.end(); ++it)
if (position > mSequence.size())
{
if (itPosition == position)
{
mSequence.insert(it, insertions.begin(), insertions.end());
return true;
}
++itPosition;
return false;
}
return false;
auto it = mSequence.begin() + position;
mSequence.insert(it, insertions.begin(), insertions.end());
return true;
}
void TIntermAggregate::setPrecisionFromChildren()
......@@ -2548,9 +2544,20 @@ void TIntermTraverser::updateTree()
{
const NodeInsertMultipleEntry &insertion = mInsertions[ii];
ASSERT(insertion.parent);
bool inserted = insertion.parent->insertChildNodes(insertion.position, insertion.insertions);
ASSERT(inserted);
UNUSED_ASSERTION_VARIABLE(inserted);
if (!insertion.insertionsAfter.empty())
{
bool inserted = insertion.parent->insertChildNodes(insertion.position + 1,
insertion.insertionsAfter);
ASSERT(inserted);
UNUSED_ASSERTION_VARIABLE(inserted);
}
if (!insertion.insertionsBefore.empty())
{
bool inserted =
insertion.parent->insertChildNodes(insertion.position, insertion.insertionsBefore);
ASSERT(inserted);
UNUSED_ASSERTION_VARIABLE(inserted);
}
}
for (size_t ii = 0; ii < mReplacements.size(); ++ii)
{
......
......@@ -781,16 +781,21 @@ class TIntermTraverser : angle::NonCopyable
// To insert multiple nodes on the parent aggregate node
struct NodeInsertMultipleEntry
{
NodeInsertMultipleEntry(TIntermAggregate *_parent, TIntermSequence::size_type _position, TIntermSequence _insertions)
NodeInsertMultipleEntry(TIntermAggregate *_parent,
TIntermSequence::size_type _position,
TIntermSequence _insertionsBefore,
TIntermSequence _insertionsAfter)
: parent(_parent),
position(_position),
insertions(_insertions)
position(_position),
insertionsBefore(_insertionsBefore),
insertionsAfter(_insertionsAfter)
{
}
TIntermAggregate *parent;
TIntermSequence::size_type position;
TIntermSequence insertions;
TIntermSequence insertionsBefore;
TIntermSequence insertionsAfter;
};
// During traversing, save all the changes that need to happen into
......@@ -807,6 +812,11 @@ class TIntermTraverser : angle::NonCopyable
// supported.
void insertStatementsInParentBlock(const TIntermSequence &insertions);
// Same as above, but supports simultaneous insertion of statements before and after the node
// currently being traversed.
void insertStatementsInParentBlock(const TIntermSequence &insertionsBefore,
const TIntermSequence &insertionsAfter);
// Helper to create a temporary symbol node with the given qualifier.
TIntermSymbol *createTempSymbol(const TType &type, TQualifier qualifier);
// Helper to create a temporary symbol node.
......
......@@ -81,8 +81,16 @@ void TIntermTraverser::popParentBlock()
void TIntermTraverser::insertStatementsInParentBlock(const TIntermSequence &insertions)
{
TIntermSequence emptyInsertionsAfter;
insertStatementsInParentBlock(insertions, emptyInsertionsAfter);
}
void TIntermTraverser::insertStatementsInParentBlock(const TIntermSequence &insertionsBefore,
const TIntermSequence &insertionsAfter)
{
ASSERT(!mParentBlockStack.empty());
NodeInsertMultipleEntry insert(mParentBlockStack.back().node, mParentBlockStack.back().pos, insertions);
NodeInsertMultipleEntry insert(mParentBlockStack.back().node, mParentBlockStack.back().pos,
insertionsBefore, insertionsAfter);
mInsertions.push_back(insert);
}
......
//
// Copyright (c) 2002-2015 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.
//
// RemoveDynamicIndexing is an AST traverser to remove dynamic indexing of vectors and matrices,
// replacing them with calls to functions that choose which component to return or write.
//
#ifndef COMPILER_TRANSLATOR_REMOVEDYNAMICINDEXING_H_
#define COMPILER_TRANSLATOR_REMOVEDYNAMICINDEXING_H_
class TIntermNode;
class TSymbolTable;
void RemoveDynamicIndexing(TIntermNode *root,
unsigned int *temporaryIndex,
const TSymbolTable &symbolTable,
int shaderVersion);
#endif // COMPILER_TRANSLATOR_REMOVEDYNAMICINDEXING_H_
......@@ -8,6 +8,7 @@
#include "compiler/translator/ArrayReturnValueToOutParameter.h"
#include "compiler/translator/OutputHLSL.h"
#include "compiler/translator/RemoveDynamicIndexing.h"
#include "compiler/translator/RewriteElseBlocks.h"
#include "compiler/translator/SeparateArrayInitialization.h"
#include "compiler/translator/SeparateDeclarations.h"
......@@ -38,6 +39,12 @@ void TranslatorHLSL::translate(TIntermNode *root, int compileOptions)
// as a return value to use an out parameter to transfer the array data instead.
ArrayReturnValueToOutParameter(root, getTemporaryIndex());
if (!shouldRunLoopAndIndexingValidation(compileOptions))
{
// HLSL doesn't support dynamic indexing of vectors and matrices.
RemoveDynamicIndexing(root, getTemporaryIndex(), getSymbolTable(), getShaderVersion());
}
// Work around D3D9 bug that would manifest in vertex shaders with selection blocks which
// use a vertex attribute as a condition, and some related computation in the else block.
if (getOutputType() == SH_HLSL9_OUTPUT && getShaderType() == GL_VERTEX_SHADER)
......
......@@ -50,18 +50,6 @@
1089 WIN : dEQP-GLES3.functional.shaders.functions.invalid.local_function_prototype_fragment = FAIL
1089 WIN : dEQP-GLES3.functional.shaders.functions.invalid.overload_builtin_function_vertex = FAIL
1089 WIN : dEQP-GLES3.functional.shaders.functions.invalid.overload_builtin_function_fragment = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.vector_subscript.vec2_dynamic_subscript_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.vector_subscript.vec3_dynamic_subscript_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.vector_subscript.vec4_dynamic_subscript_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.matrix_subscript.mat2_dynamic_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.matrix_subscript.mat2x3_dynamic_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.matrix_subscript.mat2x4_dynamic_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.matrix_subscript.mat3x2_dynamic_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.matrix_subscript.mat3_dynamic_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.matrix_subscript.mat3x4_dynamic_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.matrix_subscript.mat4x2_dynamic_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.matrix_subscript.mat4x3_dynamic_write_* = FAIL
1090 WIN : dEQP-GLES3.functional.shaders.indexing.matrix_subscript.mat4_dynamic_write_* = FAIL
1091 WIN : dEQP-GLES3.functional.shaders.loops.for_constant_iterations.nested_sequence_vertex = FAIL
1091 WIN : dEQP-GLES3.functional.shaders.loops.for_constant_iterations.nested_sequence_fragment = FAIL
1091 WIN : dEQP-GLES3.functional.shaders.loops.for_constant_iterations.nested_tricky_dataflow_1_vertex = FAIL
......
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