commit db6aa8a4add6b913f5aa4604fa26e0bafca0fa38
author Antonio Maiorano <amaiorano@google.com> Thu Nov 25 13:24:50 2021 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Nov 25 13:24:50 2021 +0000
tree d1c673c0b866fcea3c79a81bf644ca479048bcd0
parent 802292eb5249f83f1e8f990f0283b63f4f267c8a

Add "fxc_optimizations" toggle set to false by default

FXC sometimes miscompiles code when optimizing (/O2), and there is no
discernable workaround. This change sets the optimization level to /O0
when compiling shaders with FXC.

Also, no longer default to enabling EmitHLSLDebugSymbols in Debug
builds, which disabled optimizations (/Od). This confused me a few
times, and is not necessary since we can set this toggle via command
line.

Bug: dawn:1203
Bug: tint:1175
Bug: tint:1112
Change-Id: Ide9e6ecd45adeca951b8836dee91a8367eca3769
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/70700
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>

src/dawn_native/Toggles.cpp

diff --git a/src/dawn_native/Toggles.cpp b/src/dawn_native/Toggles.cpp
index 8c7f33f..b0e2dea 100644
--- a/src/dawn_native/Toggles.cpp
+++ b/src/dawn_native/Toggles.cpp
@@ -225,6 +225,12 @@
               "be enabled for OpenGL ES backend, and serves as a workaround by default enabled on "
               "some Metal devices with Intel GPU to ensure the depth result is correct.",
               "https://crbug.com/dawn/136"}},
+            {Toggle::FxcOptimizations,
+             {"fxc_optimizations",
+              "Enable optimizations when compiling with FXC. Disabled by default because FXC "
+              "miscompiles in many cases when optimizations are enabled.",
+              "https://crbug.com/dawn/1203"}},
+
             // Dummy comment to separate the }} so it is clearer what to copy-paste to add a toggle.
         }};
     }  // anonymous namespace

src/dawn_native/Toggles.h

diff --git a/src/dawn_native/Toggles.h b/src/dawn_native/Toggles.h
index 88a9d6d..4682cbd 100644
--- a/src/dawn_native/Toggles.h
+++ b/src/dawn_native/Toggles.h
@@ -60,6 +60,7 @@
         UseUserDefinedLabelsInBackend,
         DisableR8RG8Mipmaps,
         UseDummyFragmentInVertexOnlyPipeline,
+        FxcOptimizations,
 
         EnumCount,
         InvalidEnum = EnumCount,

src/dawn_native/d3d12/ComputePipelineD3D12.cpp

diff --git a/src/dawn_native/d3d12/ComputePipelineD3D12.cpp b/src/dawn_native/d3d12/ComputePipelineD3D12.cpp
index dd8d225..b6037dd 100644
--- a/src/dawn_native/d3d12/ComputePipelineD3D12.cpp
+++ b/src/dawn_native/d3d12/ComputePipelineD3D12.cpp
@@ -34,6 +34,11 @@
         Device* device = ToBackend(GetDevice());
         uint32_t compileFlags = 0;
 
+        if (!device->IsToggleEnabled(Toggle::UseDXC) &&
+            !device->IsToggleEnabled(Toggle::FxcOptimizations)) {
+            compileFlags |= D3DCOMPILE_OPTIMIZATION_LEVEL0;
+        }
+
         if (device->IsToggleEnabled(Toggle::EmitHLSLDebugSymbols)) {
             compileFlags |= D3DCOMPILE_DEBUG | D3DCOMPILE_SKIP_OPTIMIZATION;
         }

src/dawn_native/d3d12/DeviceD3D12.cpp

diff --git a/src/dawn_native/d3d12/DeviceD3D12.cpp b/src/dawn_native/d3d12/DeviceD3D12.cpp
index 8eb4a87..a9bf033 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.cpp
+++ b/src/dawn_native/d3d12/DeviceD3D12.cpp
@@ -518,10 +518,9 @@
         SetToggle(Toggle::UseD3D12ResidencyManagement, true);
         SetToggle(Toggle::UseDXC, false);
 
-#if defined(_DEBUG)
-        // Enable better shader debugging with the graphics debugging tools.
-        SetToggle(Toggle::EmitHLSLDebugSymbols, true);
-#endif
+        // Disable optimizations when using FXC
+        // See https://crbug.com/dawn/1203
+        SetToggle(Toggle::FxcOptimizations, false);
 
         // By default use the maximum shader-visible heap size allowed.
         SetToggle(Toggle::UseD3D12SmallShaderVisibleHeapForTesting, false);

src/dawn_native/d3d12/RenderPipelineD3D12.cpp

diff --git a/src/dawn_native/d3d12/RenderPipelineD3D12.cpp b/src/dawn_native/d3d12/RenderPipelineD3D12.cpp
index dbe6252..de90d1d 100644
--- a/src/dawn_native/d3d12/RenderPipelineD3D12.cpp
+++ b/src/dawn_native/d3d12/RenderPipelineD3D12.cpp
@@ -326,6 +326,11 @@
         Device* device = ToBackend(GetDevice());
         uint32_t compileFlags = 0;
 
+        if (!device->IsToggleEnabled(Toggle::UseDXC) &&
+            !device->IsToggleEnabled(Toggle::FxcOptimizations)) {
+            compileFlags |= D3DCOMPILE_OPTIMIZATION_LEVEL0;
+        }
+
         if (device->IsToggleEnabled(Toggle::EmitHLSLDebugSymbols)) {
             compileFlags |= D3DCOMPILE_DEBUG | D3DCOMPILE_SKIP_OPTIMIZATION;
         }