diff options
| author | Matt Turner <mattst88@gmail.com> | 2019-02-11 16:02:15 -0800 |
|---|---|---|
| committer | Dylan Baker <dylan@pnwbakers.com> | 2019-02-15 12:03:44 -0800 |
| commit | 4cf1a40f9a33517d9fc1bb4439528af49699661b (patch) | |
| tree | aa1e3e3ff675bb5563d1f4470c758d4a41c5be14 | |
| parent | 81e053b7578e540588e897b4d7670d0dfd8b0ab8 (diff) | |
intel/compiler: Avoid propagating inequality cmods if types are different
v2: Fix silly bug in logic. s/||/&&/
All but one of the affected shaders is in an Unreal4 demo. The other is
in Tomb Raider. All of the cases that Ian investigated appear to be
sequences like the following
if (int(uint(some_float)) < 0) /* other relations too */
...
At least in Tomb Raider, it's not obvious that this sequence came from
the original shader.
In some of the Unreal demos, the shader contains code like
if (int(uint(textureLod(...))) > 0)
...
which explicitly generates the offending sequence.
All Gen6+ platforms had similar results (Skylake shown):
total instructions in shared programs: 15437170 -> 15437187 (<.01%)
instructions in affected programs: 4492 -> 4509 (0.38%)
helped: 0
HURT: 17
HURT stats (abs) min: 1 max: 1 x̄: 1.00 x̃: 1
HURT stats (rel) min: 0.05% max: 0.73% x̄: 0.66% x̃: 0.73%
95% mean confidence interval for instructions value: 1.00 1.00
95% mean confidence interval for instructions %-change: 0.57% 0.75%
Instructions are HURT.
total cycles in shared programs: 383007996 -> 383007992 (<.01%)
cycles in affected programs: 20542 -> 20538 (-0.02%)
helped: 6
HURT: 7
helped stats (abs) min: 2 max: 6 x̄: 5.33 x̃: 6
helped stats (rel) min: 0.11% max: 0.36% x̄: 0.32% x̃: 0.36%
HURT stats (abs) min: 4 max: 4 x̄: 4.00 x̃: 4
HURT stats (rel) min: 0.27% max: 0.27% x̄: 0.27% x̃: 0.27%
95% mean confidence interval for cycles value: -3.30 2.69
95% mean confidence interval for cycles %-change: -0.19% 0.19%
Inconclusive result (value mean confidence interval includes 0).
No changes on Iron Lake or GM45.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109404
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Tested-by: nagrigoriadis@gmail.com
Tested-by: Danylo Piliaiev <danylo.piliaiev@gmail.com>
(cherry picked from commit 2dff9a66b629834bffad47e7a9025e0f1de5ffc3)
| -rw-r--r-- | src/intel/compiler/brw_fs_cmod_propagation.cpp | 7 |
1 files changed, 7 insertions, 0 deletions
diff --git a/src/intel/compiler/brw_fs_cmod_propagation.cpp b/src/intel/compiler/brw_fs_cmod_propagation.cpp index 5fb522f810f..b58730fbbe5 100644 --- a/src/intel/compiler/brw_fs_cmod_propagation.cpp +++ b/src/intel/compiler/brw_fs_cmod_propagation.cpp @@ -255,6 +255,13 @@ opt_cmod_propagation_local(const gen_device_info *devinfo, bblock_t *block) if (inst->opcode == BRW_OPCODE_AND) break; + /* Not safe to use inequality operators if the types are different + */ + if (scan_inst->dst.type != inst->src[0].type && + inst->conditional_mod != BRW_CONDITIONAL_Z && + inst->conditional_mod != BRW_CONDITIONAL_NZ) + break; + /* Comparisons operate differently for ints and floats */ if (scan_inst->dst.type != inst->dst.type && (scan_inst->dst.type == BRW_REGISTER_TYPE_F || |