Generate performance warnings in HLSL translation

src/compiler/translator/Compiler.cpp

@@ -626,7 +626,10 @@ bool TCompiler::compile(const char *const shaderStrings[],
             OutputTree(root, infoSink.info);
 
         if (compileOptions & SH_OBJECT_CODE)
-            translate(root, compileOptions);
+        {
+            PerformanceDiagnostics perfDiagnostics(&mDiagnostics);
+            translate(root, compileOptions, &perfDiagnostics);
+        }
 
         // The IntermNode tree doesn't need to be deleted here, since the
         // memory will be freed in a big chunk by the PoolAllocator.

src/compiler/translator/Compiler.h

@@ -156,8 +156,10 @@ class TCompiler : public TShHandleBase
     // Add emulated functions to the built-in function emulator.
     virtual void initBuiltInFunctionEmulator(BuiltInFunctionEmulator *emu,
                                              ShCompileOptions compileOptions){};
-    // Translate to object code.
-    virtual void translate(TIntermBlock *root, ShCompileOptions compileOptions) = 0;
+    // Translate to object code. May generate performance warnings through the diagnostics.
+    virtual void translate(TIntermBlock *root,
+                           ShCompileOptions compileOptions,
+                           PerformanceDiagnostics *perfDiagnostics) = 0;
     // Insert statements to reference all members in unused uniform blocks with standard and shared
     // layout. This is to work around a Mac driver that treats unused standard/shared
     // uniform blocks as inactive.

src/compiler/translator/Diagnostics.cpp

@@ -91,4 +91,15 @@ void TDiagnostics::resetErrorCount()
     mNumWarnings = 0;
 }
 
+PerformanceDiagnostics::PerformanceDiagnostics(TDiagnostics *diagnostics)
+    : mDiagnostics(diagnostics)
+{
+    ASSERT(diagnostics);
+}
+
+void PerformanceDiagnostics::warning(const TSourceLoc &loc, const char *reason, const char *token)
+{
+    mDiagnostics->warning(loc, reason, token);
+}
+
 }  // namespace sh

src/compiler/translator/Diagnostics.h

@@ -50,6 +50,18 @@ class TDiagnostics : public pp::Diagnostics, angle::NonCopyable
     int mNumWarnings;
 };
 
+// Diagnostics wrapper to use when the code is only allowed to generate warnings.
+class PerformanceDiagnostics : public angle::NonCopyable
+{
+  public:
+    PerformanceDiagnostics(TDiagnostics *diagnostics);
+
+    void warning(const TSourceLoc &loc, const char *reason, const char *token);
+
+  private:
+    TDiagnostics *mDiagnostics;
+};
+
 }  // namespace sh
 
 #endif  // COMPILER_TRANSLATOR_DIAGNOSTICS_H_

src/compiler/translator/OutputHLSL.cpp

@@ -121,7 +121,8 @@ OutputHLSL::OutputHLSL(sh::GLenum shaderType,
                        int numRenderTargets,
                        const std::vector<Uniform> &uniforms,
                        ShCompileOptions compileOptions,
-                       TSymbolTable *symbolTable)
+                       TSymbolTable *symbolTable,
+                       PerformanceDiagnostics *perfDiagnostics)
     : TIntermTraverser(true, true, true, symbolTable),
       mShaderType(shaderType),
       mShaderVersion(shaderVersion),
@@ -130,7 +131,8 @@ OutputHLSL::OutputHLSL(sh::GLenum shaderType,
       mOutputType(outputType),
       mCompileOptions(compileOptions),
       mNumRenderTargets(numRenderTargets),
-      mCurrentFunctionMetadata(nullptr)
+      mCurrentFunctionMetadata(nullptr),
+      mPerfDiagnostics(perfDiagnostics)
 {
     mInsideFunction = false;
 
@@ -2173,7 +2175,7 @@ bool OutputHLSL::visitSwitch(Visit visit, TIntermSwitch *node)
     ASSERT(node->getStatementList());
     if (visit == PreVisit)
     {
-        node->setStatementList(RemoveSwitchFallThrough(node->getStatementList()));
+        node->setStatementList(RemoveSwitchFallThrough(node->getStatementList(), mPerfDiagnostics));
     }
     outputTriplet(out, visit, "switch (", ") ", "");
     // The curly braces get written when visiting the statementList block.

src/compiler/translator/OutputHLSL.h

@@ -40,7 +40,8 @@ class OutputHLSL : public TIntermTraverser
                int numRenderTargets,
                const std::vector<Uniform> &uniforms,
                ShCompileOptions compileOptions,
-               TSymbolTable *symbolTable);
+               TSymbolTable *symbolTable,
+               PerformanceDiagnostics *perfDiagnostics);
 
     ~OutputHLSL();
 
@@ -241,6 +242,8 @@ class OutputHLSL : public TIntermTraverser
     // arrays can't be return values in HLSL.
     std::vector<ArrayHelperFunction> mArrayConstructIntoFunctions;
 
+    PerformanceDiagnostics *mPerfDiagnostics;
+
   private:
     TString samplerNamePrefixFromStruct(TIntermTyped *node);
     bool ancestorEvaluatesToSamplerInStruct();

src/compiler/translator/RemoveDynamicIndexing.cpp

@@ -9,6 +9,7 @@
 
 #include "compiler/translator/RemoveDynamicIndexing.h"
 
+#include "compiler/translator/Diagnostics.h"
 #include "compiler/translator/InfoSink.h"
 #include "compiler/translator/IntermNodePatternMatcher.h"
 #include "compiler/translator/IntermNode_util.h"
@@ -281,7 +282,9 @@ TIntermFunctionDefinition *GetIndexFunctionDefinition(TType type,
 class RemoveDynamicIndexingTraverser : public TLValueTrackingTraverser
 {
   public:
-    RemoveDynamicIndexingTraverser(TSymbolTable *symbolTable, int shaderVersion);
+    RemoveDynamicIndexingTraverser(TSymbolTable *symbolTable,
+                                   int shaderVersion,
+                                   PerformanceDiagnostics *perfDiagnostics);
 
     bool visitBinary(Visit visit, TIntermBinary *node) override;
 
@@ -305,13 +308,18 @@ class RemoveDynamicIndexingTraverser : public TLValueTrackingTraverser
     //   V[j++][i]++.
     // where V is an array of vectors, j++ will only be evaluated once.
     bool mRemoveIndexSideEffectsInSubtree;
+
+    PerformanceDiagnostics *mPerfDiagnostics;
 };
 
-RemoveDynamicIndexingTraverser::RemoveDynamicIndexingTraverser(TSymbolTable *symbolTable,
-                                                               int shaderVersion)
+RemoveDynamicIndexingTraverser::RemoveDynamicIndexingTraverser(
+    TSymbolTable *symbolTable,
+    int shaderVersion,
+    PerformanceDiagnostics *perfDiagnostics)
     : TLValueTrackingTraverser(true, false, false, symbolTable, shaderVersion),
       mUsedTreeInsertion(false),
-      mRemoveIndexSideEffectsInSubtree(false)
+      mRemoveIndexSideEffectsInSubtree(false),
+      mPerfDiagnostics(perfDiagnostics)
 {
 }
 
@@ -398,6 +406,10 @@ bool RemoveDynamicIndexingTraverser::visitBinary(Visit visit, TIntermBinary *nod
         }
         else if (IntermNodePatternMatcher::IsDynamicIndexingOfVectorOrMatrix(node))
         {
+            mPerfDiagnostics->warning(node->getLine(),
+                                      "Performance: dynamic indexing of vectors and "
+                                      "matrices is emulated and can be slow.",
+                                      "[]");
             bool write = isLValueRequiredHere();
 
 #if defined(ANGLE_ENABLE_ASSERTS)
@@ -515,9 +527,12 @@ void RemoveDynamicIndexingTraverser::nextIteration()
 
 }  // namespace
 
-void RemoveDynamicIndexing(TIntermNode *root, TSymbolTable *symbolTable, int shaderVersion)
+void RemoveDynamicIndexing(TIntermNode *root,
+                           TSymbolTable *symbolTable,
+                           int shaderVersion,
+                           PerformanceDiagnostics *perfDiagnostics)
 {
-    RemoveDynamicIndexingTraverser traverser(symbolTable, shaderVersion);
+    RemoveDynamicIndexingTraverser traverser(symbolTable, shaderVersion, perfDiagnostics);
     do
     {
         traverser.nextIteration();

src/compiler/translator/RemoveDynamicIndexing.h

@@ -15,8 +15,12 @@ namespace sh
 
 class TIntermNode;
 class TSymbolTable;
+class PerformanceDiagnostics;
 
-void RemoveDynamicIndexing(TIntermNode *root, TSymbolTable *symbolTable, int shaderVersion);
+void RemoveDynamicIndexing(TIntermNode *root,
+                           TSymbolTable *symbolTable,
+                           int shaderVersion,
+                           PerformanceDiagnostics *perfDiagnostics);
 
 }  // namespace sh
 

src/compiler/translator/RemoveSwitchFallThrough.cpp

@@ -10,6 +10,7 @@
 
 #include "compiler/translator/RemoveSwitchFallThrough.h"
 
+#include "compiler/translator/Diagnostics.h"
 #include "compiler/translator/IntermTraverse.h"
 
 namespace sh
@@ -21,10 +22,12 @@ namespace
 class RemoveSwitchFallThroughTraverser : public TIntermTraverser
 {
   public:
-    static TIntermBlock *removeFallThrough(TIntermBlock *statementList);
+    static TIntermBlock *removeFallThrough(TIntermBlock *statementList,
+                                           PerformanceDiagnostics *perfDiagnostics);
 
   private:
-    RemoveSwitchFallThroughTraverser(TIntermBlock *statementList);
+    RemoveSwitchFallThroughTraverser(TIntermBlock *statementList,
+                                     PerformanceDiagnostics *perfDiagnostics);
 
     void visitSymbol(TIntermSymbol *node) override;
     void visitConstantUnion(TIntermConstantUnion *node) override;
@@ -49,11 +52,14 @@ class RemoveSwitchFallThroughTraverser : public TIntermTraverser
     bool mLastStatementWasBreak;
     TIntermBlock *mPreviousCase;
     std::vector<TIntermBlock *> mCasesSharingBreak;
+    PerformanceDiagnostics *mPerfDiagnostics;
 };
 
-TIntermBlock *RemoveSwitchFallThroughTraverser::removeFallThrough(TIntermBlock *statementList)
+TIntermBlock *RemoveSwitchFallThroughTraverser::removeFallThrough(
+    TIntermBlock *statementList,
+    PerformanceDiagnostics *perfDiagnostics)
 {
-    RemoveSwitchFallThroughTraverser rm(statementList);
+    RemoveSwitchFallThroughTraverser rm(statementList, perfDiagnostics);
     ASSERT(statementList);
     statementList->traverse(&rm);
     ASSERT(rm.mPreviousCase || statementList->getSequence()->empty());
@@ -69,11 +75,14 @@ TIntermBlock *RemoveSwitchFallThroughTraverser::removeFallThrough(TIntermBlock *
     return rm.mStatementListOut;
 }
 
-RemoveSwitchFallThroughTraverser::RemoveSwitchFallThroughTraverser(TIntermBlock *statementList)
+RemoveSwitchFallThroughTraverser::RemoveSwitchFallThroughTraverser(
+    TIntermBlock *statementList,
+    PerformanceDiagnostics *perfDiagnostics)
     : TIntermTraverser(true, false, false),
       mStatementList(statementList),
       mLastStatementWasBreak(false),
-      mPreviousCase(nullptr)
+      mPreviousCase(nullptr),
+      mPerfDiagnostics(perfDiagnostics)
 {
     mStatementListOut = new TIntermBlock();
 }
@@ -158,14 +167,10 @@ void RemoveSwitchFallThroughTraverser::handlePreviousCase()
         mCasesSharingBreak.push_back(mPreviousCase);
     if (mLastStatementWasBreak)
     {
-        bool labelsWithNoStatements = true;
         for (size_t i = 0; i < mCasesSharingBreak.size(); ++i)
         {
-            if (mCasesSharingBreak.at(i)->getSequence()->size() > 1)
-            {
-                labelsWithNoStatements = false;
-            }
-            if (labelsWithNoStatements)
+            ASSERT(!mCasesSharingBreak.at(i)->getSequence()->empty());
+            if (mCasesSharingBreak.at(i)->getSequence()->size() == 1)
             {
                 // Fall-through is allowed in case the label has no statements.
                 outputSequence(mCasesSharingBreak.at(i)->getSequence(), 0);
@@ -173,6 +178,13 @@ void RemoveSwitchFallThroughTraverser::handlePreviousCase()
             else
             {
                 // Include all the statements that this case can fall through under the same label.
+                if (mCasesSharingBreak.size() > i + 1u)
+                {
+                    mPerfDiagnostics->warning(mCasesSharingBreak.at(i)->getLine(),
+                                              "Performance: non-empty fall-through cases in "
+                                              "switch statements generate extra code.",
+                                              "switch");
+                }
                 for (size_t j = i; j < mCasesSharingBreak.size(); ++j)
                 {
                     size_t startIndex =
@@ -192,6 +204,7 @@ bool RemoveSwitchFallThroughTraverser::visitCase(Visit, TIntermCase *node)
     handlePreviousCase();
     mPreviousCase = new TIntermBlock();
     mPreviousCase->getSequence()->push_back(node);
+    mPreviousCase->setLine(node->getLine());
     // Don't traverse the condition of the case statement
     return false;
 }
@@ -248,9 +261,10 @@ bool RemoveSwitchFallThroughTraverser::visitBranch(Visit, TIntermBranch *node)
 
 }  // anonymous namespace
 
-TIntermBlock *RemoveSwitchFallThrough(TIntermBlock *statementList)
+TIntermBlock *RemoveSwitchFallThrough(TIntermBlock *statementList,
+                                      PerformanceDiagnostics *perfDiagnostics)
 {
-    return RemoveSwitchFallThroughTraverser::removeFallThrough(statementList);
+    return RemoveSwitchFallThroughTraverser::removeFallThrough(statementList, perfDiagnostics);
 }
 
 }  // namespace sh

src/compiler/translator/RemoveSwitchFallThrough.h

@@ -15,10 +15,12 @@ namespace sh
 {
 
 class TIntermBlock;
+class PerformanceDiagnostics;
 
 // When given a statementList from a switch AST node, return an updated
 // statementList that has fall-through removed.
-TIntermBlock *RemoveSwitchFallThrough(TIntermBlock *statementList);
+TIntermBlock *RemoveSwitchFallThrough(TIntermBlock *statementList,
+                                      PerformanceDiagnostics *perfDiagnostics);
 
 }  // namespace sh
 

src/compiler/translator/TranslatorESSL.cpp

@@ -30,7 +30,9 @@ void TranslatorESSL::initBuiltInFunctionEmulator(BuiltInFunctionEmulator *emu,
     }
 }
 
-void TranslatorESSL::translate(TIntermBlock *root, ShCompileOptions compileOptions)
+void TranslatorESSL::translate(TIntermBlock *root,
+                               ShCompileOptions compileOptions,
+                               PerformanceDiagnostics * /*perfDiagnostics*/)
 {
     // The ESSL output doesn't define a default precision for float, so float literal statements
     // end up with no precision which is invalid ESSL.

src/compiler/translator/TranslatorESSL.h

@@ -21,7 +21,9 @@ class TranslatorESSL : public TCompiler
     void initBuiltInFunctionEmulator(BuiltInFunctionEmulator *emu,
                                      ShCompileOptions compileOptions) override;
 
-    void translate(TIntermBlock *root, ShCompileOptions compileOptions) override;
+    void translate(TIntermBlock *root,
+                   ShCompileOptions compileOptions,
+                   PerformanceDiagnostics *perfDiagnostics) override;
     bool shouldFlattenPragmaStdglInvariantAll() override;
 
   private:

src/compiler/translator/TranslatorGLSL.cpp

@@ -45,7 +45,9 @@ void TranslatorGLSL::initBuiltInFunctionEmulator(BuiltInFunctionEmulator *emu,
     InitBuiltInFunctionEmulatorForGLSLMissingFunctions(emu, getShaderType(), targetGLSLVersion);
 }
 
-void TranslatorGLSL::translate(TIntermBlock *root, ShCompileOptions compileOptions)
+void TranslatorGLSL::translate(TIntermBlock *root,
+                               ShCompileOptions compileOptions,
+                               PerformanceDiagnostics * /*perfDiagnostics*/)
 {
     TInfoSinkBase &sink = getInfoSink().obj;
 

src/compiler/translator/TranslatorGLSL.h

@@ -21,7 +21,9 @@ class TranslatorGLSL : public TCompiler
     void initBuiltInFunctionEmulator(BuiltInFunctionEmulator *emu,
                                      ShCompileOptions compileOptions) override;
 
-    void translate(TIntermBlock *root, ShCompileOptions compileOptions) override;
+    void translate(TIntermBlock *root,
+                   ShCompileOptions compileOptions,
+                   PerformanceDiagnostics *perfDiagnostics) override;
     bool shouldFlattenPragmaStdglInvariantAll() override;
     bool shouldCollectVariables(ShCompileOptions compileOptions) override;
 

src/compiler/translator/TranslatorHLSL.cpp

@@ -34,7 +34,9 @@ TranslatorHLSL::TranslatorHLSL(sh::GLenum type, ShShaderSpec spec, ShShaderOutpu
 {
 }
 
-void TranslatorHLSL::translate(TIntermBlock *root, ShCompileOptions compileOptions)
+void TranslatorHLSL::translate(TIntermBlock *root,
+                               ShCompileOptions compileOptions,
+                               PerformanceDiagnostics *perfDiagnostics)
 {
     const ShBuiltInResources &resources = getResources();
     int numRenderTargets                = resources.EXT_draw_buffers ? resources.MaxDrawBuffers : 1;
@@ -71,7 +73,7 @@ void TranslatorHLSL::translate(TIntermBlock *root, ShCompileOptions compileOptio
     if (!shouldRunLoopAndIndexingValidation(compileOptions))
     {
         // HLSL doesn't support dynamic indexing of vectors and matrices.
-        RemoveDynamicIndexing(root, &getSymbolTable(), getShaderVersion());
+        RemoveDynamicIndexing(root, &getSymbolTable(), getShaderVersion(), perfDiagnostics);
     }
 
     // Work around D3D9 bug that would manifest in vertex shaders with selection blocks which
@@ -126,7 +128,7 @@ void TranslatorHLSL::translate(TIntermBlock *root, ShCompileOptions compileOptio
 
     sh::OutputHLSL outputHLSL(getShaderType(), getShaderVersion(), getExtensionBehavior(),
                               getSourcePath(), getOutputType(), numRenderTargets, getUniforms(),
-                              compileOptions, &getSymbolTable());
+                              compileOptions, &getSymbolTable(), perfDiagnostics);
 
     outputHLSL.output(root, getInfoSink().obj);
 

src/compiler/translator/TranslatorHLSL.h

@@ -24,7 +24,9 @@ class TranslatorHLSL : public TCompiler
     const std::map<std::string, unsigned int> *getUniformRegisterMap() const;
 
   protected:
-    void translate(TIntermBlock *root, ShCompileOptions compileOptions) override;
+    void translate(TIntermBlock *root,
+                   ShCompileOptions compileOptions,
+                   PerformanceDiagnostics *perfDiagnostics) override;
     bool shouldFlattenPragmaStdglInvariantAll() override;
 
     // collectVariables needs to be run always so registers can be assigned.

src/compiler/translator/TranslatorVulkan.cpp

@@ -94,7 +94,9 @@ TranslatorVulkan::TranslatorVulkan(sh::GLenum type, ShShaderSpec spec)
 {
 }
 
-void TranslatorVulkan::translate(TIntermBlock *root, ShCompileOptions compileOptions)
+void TranslatorVulkan::translate(TIntermBlock *root,
+                                 ShCompileOptions compileOptions,
+                                 PerformanceDiagnostics * /*perfDiagnostics*/)
 {
     TInfoSinkBase &sink = getInfoSink().obj;
 

src/compiler/translator/TranslatorVulkan.h

@@ -23,7 +23,9 @@ class TranslatorVulkan : public TCompiler
     TranslatorVulkan(sh::GLenum type, ShShaderSpec spec);
 
   protected:
-    void translate(TIntermBlock *root, ShCompileOptions compileOptions) override;
+    void translate(TIntermBlock *root,
+                   ShCompileOptions compileOptions,
+                   PerformanceDiagnostics *perfDiagnostics) override;
     bool shouldFlattenPragmaStdglInvariantAll() override;
 };