i965/fs: Update discard jump to preserve uniform loads via sampler.

Commit 17c7ead7 exposed a bug in how uniform loading happens in the
presence of discard.  It manifested itself in an application as
randomly incorrect pixels on the borders of conditional areas.

This is due to how discards jump to the end of the shader incorrectly
for some channels.  The current implementation checks each 2x2
subspan to preserve derivatives.  When uniform loading via samplers
was turned on, it uses a full execution mask, as stated in
lower_uniform_pull_constant_loads(), and only populates four channels
of the destination (see generate_uniform_pull_constant_load_gen7()).
It happens incorrectly when the first subspan has been jumped over.

The series that implemented this optimization was done before the
changes to use samplers for uniform loads.  Uniform sampler loads
use special execution masks and only populate four channels, so we
can't jump over those or corruption ensues.

This fix only jumps to the end of the shader if all relevant channels
are disabled, i.e. all 8 or 16, depending on dispatch.  This
preserves the original GLbenchmark 2.7 speedup noted in commit
beafced2.

It changes the shader assembly accordingly:

before   : (-f0.1.any4h)  halt(8)  17 2  null { align1 WE_all 1Q };
after(8) : (-f0.1.any8h)  halt(8)  17 2  null { align1 WE_all 1Q };
after(16): (-f0.1.any16h) halt(16) 17 2  null { align1 WE_all 1H };

v2: Cleaned up comments and conditional ordering.
v3: Fix typo.

Signed-off-by: Cody Northrop <cody@lunarg.com>
Reviewed-by: Mike Stroyan <mike@lunarg.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79948

diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 6d9c31f2336508f6a54f96fd6d8b32edfe568dd0..cfa67925e4a6b429a0a26ec569dea8c1f0e3659b 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -1919,15 +1919,14 @@ fs_visitor::visit(ir_discard *ir)
 
    if (brw->gen >= 6) {
       /* For performance, after a discard, jump to the end of the shader.
-       * However, many people will do foliage by discarding based on a
-       * texture's alpha mask, and then continue on to texture with the
-       * remaining pixels.  To avoid trashing the derivatives for those
-       * texture samples, we'll only jump if all of the pixels in the subspan
-       * have been discarded.
+       * Only jump if all relevant channels have been discarded.
        */
       fs_inst *discard_jump = emit(FS_OPCODE_DISCARD_JUMP);
       discard_jump->flag_subreg = 1;
-      discard_jump->predicate = BRW_PREDICATE_ALIGN1_ANY4H;
+
+      discard_jump->predicate = (dispatch_width == 8)
+                                ? BRW_PREDICATE_ALIGN1_ANY8H
+                                : BRW_PREDICATE_ALIGN1_ANY16H;
       discard_jump->predicate_inverse = true;
    }
 }