Commit 5d91dda9 by Olli Etuaho Committed by Geoff Lang

Remove dynamic indexing of matrices and vectors in HLSL

Re-re-relanding after clang warning fix. 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: I9f6d7c7ecda8ac4dc3c30b39e15a9a0b5381c5a8 Reviewed-on: https://chromium-review.googlesource.com/310010Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Tested-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent 4e31ad55
...@@ -159,6 +159,8 @@ ...@@ -159,6 +159,8 @@
'compiler/translator/BuiltInFunctionEmulatorHLSL.h', 'compiler/translator/BuiltInFunctionEmulatorHLSL.h',
'compiler/translator/OutputHLSL.cpp', 'compiler/translator/OutputHLSL.cpp',
'compiler/translator/OutputHLSL.h', 'compiler/translator/OutputHLSL.h',
'compiler/translator/RemoveDynamicIndexing.cpp',
'compiler/translator/RemoveDynamicIndexing.h',
'compiler/translator/RemoveSwitchFallThrough.cpp', 'compiler/translator/RemoveSwitchFallThrough.cpp',
'compiler/translator/RemoveSwitchFallThrough.h', 'compiler/translator/RemoveSwitchFallThrough.h',
'compiler/translator/RewriteElseBlocks.cpp', 'compiler/translator/RewriteElseBlocks.cpp',
......
...@@ -153,6 +153,15 @@ TCompiler::~TCompiler() ...@@ -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) bool TCompiler::Init(const ShBuiltInResources& resources)
{ {
shaderVersion = 100; shaderVersion = 100;
...@@ -230,12 +239,6 @@ TIntermNode *TCompiler::compileTreeImpl(const char *const shaderStrings[], ...@@ -230,12 +239,6 @@ TIntermNode *TCompiler::compileTreeImpl(const char *const shaderStrings[],
success = false; 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; TIntermNode *root = nullptr;
if (success) if (success)
...@@ -281,7 +284,7 @@ TIntermNode *TCompiler::compileTreeImpl(const char *const shaderStrings[], ...@@ -281,7 +284,7 @@ TIntermNode *TCompiler::compileTreeImpl(const char *const shaderStrings[],
if (success && shaderVersion == 300 && shaderType == GL_FRAGMENT_SHADER) if (success && shaderVersion == 300 && shaderType == GL_FRAGMENT_SHADER)
success = validateOutputs(root); success = validateOutputs(root);
if (success && validateLoopAndIndexing) if (success && shouldRunLoopAndIndexingValidation(compileOptions))
success = validateLimitations(root); success = validateLimitations(root);
if (success && (compileOptions & SH_TIMING_RESTRICTIONS)) if (success && (compileOptions & SH_TIMING_RESTRICTIONS))
......
...@@ -101,6 +101,8 @@ class TCompiler : public TShHandleBase ...@@ -101,6 +101,8 @@ class TCompiler : public TShHandleBase
ShShaderOutput getOutputType() const { return outputType; } ShShaderOutput getOutputType() const { return outputType; }
const std::string &getBuiltInResourcesString() const { return builtInResourcesString; } const std::string &getBuiltInResourcesString() const { return builtInResourcesString; }
bool shouldRunLoopAndIndexingValidation(int compileOptions) const;
// Get the resources set by InitBuiltInSymbolTable // Get the resources set by InitBuiltInSymbolTable
const ShBuiltInResources& getResources() const; const ShBuiltInResources& getResources() const;
......
...@@ -311,17 +311,13 @@ bool TIntermAggregate::replaceChildNodeWithMultiple(TIntermNode *original, TInte ...@@ -311,17 +311,13 @@ bool TIntermAggregate::replaceChildNodeWithMultiple(TIntermNode *original, TInte
bool TIntermAggregate::insertChildNodes(TIntermSequence::size_type position, TIntermSequence insertions) bool TIntermAggregate::insertChildNodes(TIntermSequence::size_type position, TIntermSequence insertions)
{ {
TIntermSequence::size_type itPosition = 0; if (position > mSequence.size())
for (auto it = mSequence.begin(); it < mSequence.end(); ++it)
{ {
if (itPosition == position) return false;
{
mSequence.insert(it, insertions.begin(), insertions.end());
return true;
}
++itPosition;
} }
return false; auto it = mSequence.begin() + position;
mSequence.insert(it, insertions.begin(), insertions.end());
return true;
} }
void TIntermAggregate::setPrecisionFromChildren() void TIntermAggregate::setPrecisionFromChildren()
...@@ -2548,9 +2544,20 @@ void TIntermTraverser::updateTree() ...@@ -2548,9 +2544,20 @@ void TIntermTraverser::updateTree()
{ {
const NodeInsertMultipleEntry &insertion = mInsertions[ii]; const NodeInsertMultipleEntry &insertion = mInsertions[ii];
ASSERT(insertion.parent); ASSERT(insertion.parent);
bool inserted = insertion.parent->insertChildNodes(insertion.position, insertion.insertions); if (!insertion.insertionsAfter.empty())
ASSERT(inserted); {
UNUSED_ASSERTION_VARIABLE(inserted); 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) for (size_t ii = 0; ii < mReplacements.size(); ++ii)
{ {
......
...@@ -781,16 +781,21 @@ class TIntermTraverser : angle::NonCopyable ...@@ -781,16 +781,21 @@ class TIntermTraverser : angle::NonCopyable
// To insert multiple nodes on the parent aggregate node // To insert multiple nodes on the parent aggregate node
struct NodeInsertMultipleEntry struct NodeInsertMultipleEntry
{ {
NodeInsertMultipleEntry(TIntermAggregate *_parent, TIntermSequence::size_type _position, TIntermSequence _insertions) NodeInsertMultipleEntry(TIntermAggregate *_parent,
TIntermSequence::size_type _position,
TIntermSequence _insertionsBefore,
TIntermSequence _insertionsAfter)
: parent(_parent), : parent(_parent),
position(_position), position(_position),
insertions(_insertions) insertionsBefore(_insertionsBefore),
insertionsAfter(_insertionsAfter)
{ {
} }
TIntermAggregate *parent; TIntermAggregate *parent;
TIntermSequence::size_type position; TIntermSequence::size_type position;
TIntermSequence insertions; TIntermSequence insertionsBefore;
TIntermSequence insertionsAfter;
}; };
// During traversing, save all the changes that need to happen into // During traversing, save all the changes that need to happen into
...@@ -807,6 +812,11 @@ class TIntermTraverser : angle::NonCopyable ...@@ -807,6 +812,11 @@ class TIntermTraverser : angle::NonCopyable
// supported. // supported.
void insertStatementsInParentBlock(const TIntermSequence &insertions); 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. // Helper to create a temporary symbol node with the given qualifier.
TIntermSymbol *createTempSymbol(const TType &type, TQualifier qualifier); TIntermSymbol *createTempSymbol(const TType &type, TQualifier qualifier);
// Helper to create a temporary symbol node. // Helper to create a temporary symbol node.
......
...@@ -81,8 +81,16 @@ void TIntermTraverser::popParentBlock() ...@@ -81,8 +81,16 @@ void TIntermTraverser::popParentBlock()
void TIntermTraverser::insertStatementsInParentBlock(const TIntermSequence &insertions) void TIntermTraverser::insertStatementsInParentBlock(const TIntermSequence &insertions)
{ {
TIntermSequence emptyInsertionsAfter;
insertStatementsInParentBlock(insertions, emptyInsertionsAfter);
}
void TIntermTraverser::insertStatementsInParentBlock(const TIntermSequence &insertionsBefore,
const TIntermSequence &insertionsAfter)
{
ASSERT(!mParentBlockStack.empty()); 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); 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 @@ ...@@ -8,6 +8,7 @@
#include "compiler/translator/ArrayReturnValueToOutParameter.h" #include "compiler/translator/ArrayReturnValueToOutParameter.h"
#include "compiler/translator/OutputHLSL.h" #include "compiler/translator/OutputHLSL.h"
#include "compiler/translator/RemoveDynamicIndexing.h"
#include "compiler/translator/RewriteElseBlocks.h" #include "compiler/translator/RewriteElseBlocks.h"
#include "compiler/translator/SeparateArrayInitialization.h" #include "compiler/translator/SeparateArrayInitialization.h"
#include "compiler/translator/SeparateDeclarations.h" #include "compiler/translator/SeparateDeclarations.h"
...@@ -38,6 +39,12 @@ void TranslatorHLSL::translate(TIntermNode *root, int compileOptions) ...@@ -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. // as a return value to use an out parameter to transfer the array data instead.
ArrayReturnValueToOutParameter(root, getTemporaryIndex()); 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 // 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. // use a vertex attribute as a condition, and some related computation in the else block.
if (getOutputType() == SH_HLSL9_OUTPUT && getShaderType() == GL_VERTEX_SHADER) if (getOutputType() == SH_HLSL9_OUTPUT && getShaderType() == GL_VERTEX_SHADER)
......
...@@ -50,18 +50,6 @@ ...@@ -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.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_vertex = FAIL
1089 WIN : dEQP-GLES3.functional.shaders.functions.invalid.overload_builtin_function_fragment = 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_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_sequence_fragment = FAIL
1091 WIN : dEQP-GLES3.functional.shaders.loops.for_constant_iterations.nested_tricky_dataflow_1_vertex = 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