intel/compiler: Use CMPN for min / max on Gen4 and Gen5

On Intel platforms before Gen6, there is no min or max instruction.
Instead, a comparison instruction (*more on this below) and a SEL
instruction are used.  Per other IEEE rules, the regular comparison
instruction, CMP, will always return false if either source is NaN.  A
sequence like

    cmp.l.f0.0(16)  null<1>F        g30<8,8,1>F     g22<8,8,1>F
    (+f0.0) sel(16) g8<1>F          g30<8,8,1>F     g22<8,8,1>F

will generate the wrong result for min if g22 is NaN.  The CMP will
return false, and the SEL will pick g22.

To account for this, the hardware has a special comparison instruction
CMPN.  This instruction behaves just like CMP, except if the second
source is NaN, it will return true.  The intention is to use it for min
and max.  This sequence will always generate the correct result:

    cmpn.l.f0.0(16) null<1>F        g30<8,8,1>F     g22<8,8,1>F
    (+f0.0) sel(16) g8<1>F          g30<8,8,1>F     g22<8,8,1>F

The problem is... for whatever reason, we don't emit CMPN.  There was
even a comment in lower_minmax that calls out this very issue!  The bug
is actually older than the "Fixes" below even implies.  That's just when
the comment was added.  That we know of, we never observed a failure
until #4254.

If src1 is known to be a number, either because it's not float or it's
an immediate number, use CMP.  This allows cmod propagation to still do
its thing.  Without this slight optimization, about 8,300 shaders from
shader-db are hurt on Iron Lake.

Fixes the following piglit tests (from piglit!475):

    tests/spec/glsl-1.20/execution/fs-nan-builtin-max.shader_test
    tests/spec/glsl-1.20/execution/fs-nan-builtin-min.shader_test
    tests/spec/glsl-1.20/execution/vs-nan-builtin-max.shader_test
    tests/spec/glsl-1.20/execution/vs-nan-builtin-min.shader_test

Closes: #4254
Fixes: 2f2c00c727 ("i965: Lower min/max after optimization on Gen4/5.")
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

Iron Lake and GM45 had similar results. (Iron Lake shown)
total instructions in shared programs: 8115134 -> 8115135 (<.01%)
instructions in affected programs: 229 -> 230 (0.44%)
helped: 0

HURT: 1
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9027>
This commit is contained in:
Ian Romanick
2021-02-13 14:11:58 -08:00
committed by Marge Bot
parent 684ec33c79
commit 3c31364f5e
2 changed files with 24 additions and 8 deletions

View File

@@ -4258,11 +4258,19 @@ fs_visitor::lower_minmax()
if (inst->opcode == BRW_OPCODE_SEL &&
inst->predicate == BRW_PREDICATE_NONE) {
/* FIXME: Using CMP doesn't preserve the NaN propagation semantics of
* the original SEL.L/GE instruction
/* If src1 is an immediate value that is not NaN, then it can't be
* NaN. In that case, emit CMP because it is much better for cmod
* propagation. Likewise if src1 is not float. Gen4 and Gen5 don't
* support HF or DF, so it is not necessary to check for those.
*/
ibld.CMP(ibld.null_reg_d(), inst->src[0], inst->src[1],
inst->conditional_mod);
if (inst->src[1].type != BRW_REGISTER_TYPE_F ||
(inst->src[1].file == IMM && !isnan(inst->src[1].f))) {
ibld.CMP(ibld.null_reg_d(), inst->src[0], inst->src[1],
inst->conditional_mod);
} else {
ibld.CMPN(ibld.null_reg_d(), inst->src[0], inst->src[1],
inst->conditional_mod);
}
inst->predicate = BRW_PREDICATE_NORMAL;
inst->conditional_mod = BRW_CONDITIONAL_NONE;

View File

@@ -1869,11 +1869,19 @@ vec4_visitor::lower_minmax()
if (inst->opcode == BRW_OPCODE_SEL &&
inst->predicate == BRW_PREDICATE_NONE) {
/* FIXME: Using CMP doesn't preserve the NaN propagation semantics of
* the original SEL.L/GE instruction
/* If src1 is an immediate value that is not NaN, then it can't be
* NaN. In that case, emit CMP because it is much better for cmod
* propagation. Likewise if src1 is not float. Gen4 and Gen5 don't
* support HF or DF, so it is not necessary to check for those.
*/
ibld.CMP(ibld.null_reg_d(), inst->src[0], inst->src[1],
inst->conditional_mod);
if (inst->src[1].type != BRW_REGISTER_TYPE_F ||
(inst->src[1].file == IMM && !isnan(inst->src[1].f))) {
ibld.CMP(ibld.null_reg_d(), inst->src[0], inst->src[1],
inst->conditional_mod);
} else {
ibld.CMPN(ibld.null_reg_d(), inst->src[0], inst->src[1],
inst->conditional_mod);
}
inst->predicate = BRW_PREDICATE_NORMAL;
inst->conditional_mod = BRW_CONDITIONAL_NONE;