Merge pull request #1320 from greg-lunarg/addopt21

Only print legalization warning if optimizer not enabled

CMakeLists.txt

@@ -94,9 +94,10 @@ endif()
 
 if(ENABLE_OPT)
     message(STATUS "optimizer enabled")
-    add_definitions(-DENABLE_OPT)
+    add_definitions(-DENABLE_OPT=1)
 elseif(ENABLE_HLSL)
     message(STATUS "spirv-tools not linked - illegal SPIRV may be generated for HLSL")
+    add_definitions(-DENABLE_OPT=0)
 endif()
 
 add_subdirectory(glslang)

SPIRV/GlslangToSpv.cpp

@@ -54,12 +54,12 @@ namespace spv {
 #endif
 }
 
-#ifdef ENABLE_OPT
+#if ENABLE_OPT
     #include "spirv-tools/optimizer.hpp"
     #include "message.h"
 #endif
 
-#ifdef ENABLE_OPT
+#if ENABLE_OPT
 using namespace spvtools;
 #endif
 
@@ -6757,7 +6757,7 @@ void GlslangToSpv(const glslang::TIntermediate& intermediate, std::vector<unsign
     it.finishSpv();
     it.dumpSpv(spirv);
 
-#ifdef ENABLE_OPT
+#if ENABLE_OPT
     // If from HLSL, run spirv-opt to "legalize" the SPIR-V for Vulkan
     // eg. forward and remove memory writes of opaque types.
     if ((intermediate.getSource() == EShSourceHlsl ||

StandAlone/StandAlone.cpp

@@ -574,7 +574,7 @@ void ProcessArguments(std::vector<std::unique_ptr<glslang::TWorkItem>>& workItem
                 if (argv[0][2] == 'd')
                     Options |= EOptionOptimizeDisable;
                 else if (argv[0][2] == 's')
-#ifdef ENABLE_OPT
+#if ENABLE_OPT
                     Options |= EOptionOptimizeSize;
 #else
                     Error("-Os not available; optimizer not linked");
@@ -720,6 +720,8 @@ void SetMessageOptions(EShMessages& messages)
         messages = (EShMessages)(messages | EShMsgDebugInfo);
     if (HlslEnable16BitTypes)
         messages = (EShMessages)(messages | EShMsgHlslEnable16BitTypes);
+    if ((Options & EOptionOptimizeDisable) || !ENABLE_OPT)
+        messages = (EShMessages)(messages | EShMsgHlslLegalization);
 }
 
 //

Test/baseLegalResults/hlsl.aliasOpaque.frag.out

@@ -1,5 +1,4 @@
 hlsl.aliasOpaque.frag
-WARNING: AST will form illegal SPIR-V; need to transform to legalize
 // Module Version 10000
 // Generated by (magic number): 80006
 // Id's are bound by 87

Test/baseLegalResults/hlsl.flattenOpaque.frag.out

@@ -1,5 +1,4 @@
 hlsl.flattenOpaque.frag
-WARNING: AST will form illegal SPIR-V; need to transform to legalize
 // Module Version 10000
 // Generated by (magic number): 80006
 // Id's are bound by 185

Test/baseLegalResults/hlsl.flattenOpaqueInit.vert.out

@@ -1,5 +1,4 @@
 hlsl.flattenOpaqueInit.vert
-WARNING: AST will form illegal SPIR-V; need to transform to legalize
 // Module Version 10000
 // Generated by (magic number): 80006
 // Id's are bound by 134

Test/baseLegalResults/hlsl.flattenOpaqueInitMix.vert.out

@@ -1,5 +1,4 @@
 hlsl.flattenOpaqueInitMix.vert
-WARNING: AST will form illegal SPIR-V; need to transform to legalize
 // Module Version 10000
 // Generated by (magic number): 80006
 // Id's are bound by 97

Test/baseLegalResults/hlsl.flattenSubset.frag.out

@@ -1,5 +1,4 @@
 hlsl.flattenSubset.frag
-WARNING: AST will form illegal SPIR-V; need to transform to legalize
 // Module Version 10000
 // Generated by (magic number): 80006
 // Id's are bound by 66

Test/baseLegalResults/hlsl.flattenSubset2.frag.out

@@ -1,5 +1,4 @@
 hlsl.flattenSubset2.frag
-WARNING: AST will form illegal SPIR-V; need to transform to legalize
 // Module Version 10000
 // Generated by (magic number): 80006
 // Id's are bound by 53

Test/baseLegalResults/hlsl.partialFlattenLocal.vert.out

@@ -1,5 +1,4 @@
 hlsl.partialFlattenLocal.vert
-WARNING: AST will form illegal SPIR-V; need to transform to legalize
 // Module Version 10000
 // Generated by (magic number): 80006
 // Id's are bound by 169

Test/baseLegalResults/hlsl.partialFlattenMixed.vert.out

@@ -1,5 +1,4 @@
 hlsl.partialFlattenMixed.vert
-WARNING: AST will form illegal SPIR-V; need to transform to legalize
 // Module Version 10000
 // Generated by (magic number): 80006
 // Id's are bound by 36

glslang/Public/ShaderLang.h

@@ -215,6 +215,7 @@ enum EShMessages {
     EShMsgHlslOffsets      = (1 << 9),  // allow block offsets to follow HLSL rules instead of GLSL rules
     EShMsgDebugInfo        = (1 << 10), // save debug information
     EShMsgHlslEnable16BitTypes  = (1 << 11), // enable use of 16-bit types in SPIR-V for HLSL
+    EShMsgHlslLegalization  = (1 << 12), // enable HLSL Legalization messages
 };
 
 //

gtests/Hlsl.FromFile.cpp

@@ -92,7 +92,7 @@ TEST_P(HlslLegalizeTest, FromFile)
     loadFileCompileAndCheck(GlobalTestSettings.testRoot, GetParam().fileName,
                             Source::HLSL, Semantics::Vulkan, glslang::EShTargetVulkan_1_0,
                             Target::Spv, true, GetParam().entryPoint,
-                            "/baseLegalResults/", false);
+                            "/baseLegalResults/", true);
 }
 
 // clang-format off
@@ -410,7 +410,7 @@ INSTANTIATE_TEST_CASE_P(
 );
 // clang-format on
 
-#ifdef ENABLE_OPT
+#if ENABLE_OPT
 // clang-format off
 INSTANTIATE_TEST_CASE_P(
     ToSpirv, HlslLegalizeTest,

gtests/TestFixture.cpp

@@ -100,6 +100,8 @@ EShMessages DeriveOptions(Source source, Semantics semantics, Target target)
             break;
     }
 
+    result = static_cast<EShMessages>(result | EShMsgHlslLegalization);
+
     return result;
 }
 

gtests/TestFixture.h

@@ -200,7 +200,7 @@ public:
             glslang::EShTargetClientVersion clientTargetVersion,
             bool flattenUniformArrays = false,
             EShTextureSamplerTransformMode texSampTransMode = EShTexSampTransKeep,
-            bool disableOptimizer = true,
+            bool enableOptimizer = false,
             bool automap = true)
     {
         const EShLanguage stage = GetShaderStage(GetSuffix(shaderName));
@@ -242,7 +242,7 @@ public:
         if (success && (controls & EShMsgSpvRules)) {
             std::vector<uint32_t> spirv_binary;
             glslang::SpvOptions options;
-            options.disableOptimizer = disableOptimizer;
+            options.disableOptimizer = !enableOptimizer;
             glslang::GlslangToSpv(*program.getIntermediate(stage),
                                   spirv_binary, &logger, &options);
 
@@ -412,7 +412,7 @@ public:
                                  bool automap = true,
                                  const std::string& entryPointName="",
                                  const std::string& baseDir="/baseResults/",
-                                 const bool disableOptimizer = true)
+                                 const bool enableOptimizer = false)
     {
         const std::string inputFname = testDir + "/" + testName;
         const std::string expectedOutputFname =
@@ -422,9 +422,11 @@ public:
         tryLoadFile(inputFname, "input", &input);
         tryLoadFile(expectedOutputFname, "expected output", &expectedOutput);
 
-        const EShMessages controls = DeriveOptions(source, semantics, target);
+        EShMessages controls = DeriveOptions(source, semantics, target);
+        if (enableOptimizer)
+            controls = static_cast<EShMessages>(controls & ~EShMsgHlslLegalization);
         GlslangResult result = compileAndLink(testName, input, entryPointName, controls, clientTargetVersion, false,
-                                              EShTexSampTransKeep, disableOptimizer, automap);
+                                              EShTexSampTransKeep, enableOptimizer, automap);
 
         // Generate the hybrid output in the way of glslangValidator.
         std::ostringstream stream;

hlsl/hlslParseHelper.cpp

@@ -9924,7 +9924,7 @@ void HlslParseContext::finish()
 
     // Communicate out (esp. for command line) that we formed AST that will make
     // illegal AST SPIR-V and it needs transforms to legalize it.
-    if (intermediate.needsLegalization())
+    if (intermediate.needsLegalization() && (messages & EShMsgHlslLegalization))
         infoSink.info << "WARNING: AST will form illegal SPIR-V; need to transform to legalize";
 
     TParseContextBase::finish();