This won't work, since this optimization in nir_opt_algebraic will undo it:
# For any float comparison operation, "cmp", if you have "a == a && a cmp b"
# then the "a == a" is redundant because it's equivalent to "a is not NaN"
# and, if a is a NaN then the second comparison will fail anyway.
for op in ['flt', 'fge', 'feq']:
optimizations += [
(('iand', ('feq', a, a), (op, a, b)), (op, a, b)),
(('iand', ('feq', a, a), (op, b, a)), (op, b, a)),
]
and then this optimization might change the behavior for NaN's:
# Comparison simplifications
(('~inot', ('flt', a, b)), ('fge', a, b)),
(('~inot', ('fge', a, b)), ('flt', a, b)),
(('~inot', ('feq', a, b)), ('fne', a, b)),
(('~inot', ('fne', a, b)), ('feq', a, b)),
The topic of NaN's and comparisons in NIR has been discussed several
times before, most recently with this thread:
https://lists.freedesktop.org/archives/mesa-dev/2018-December/210780.html
Given that this language got added to the Vulkan spec: "By default,
the implementation may perform optimizations on half, single, or
double-precision floating-point instructions respectively that ignore
sign of a zero, or assume that arguments and results are not Nans or
±∞" I think we should probably do the following:
- Fix the CTS tests that prompted this whole block of code to only
check the result of comparing NaN when this extension is available and
NaN's are preserved.
- nir_op_fne must be unordered already, regardless of what
floating-point options the user asked for, since it's used to
implement isNan() already. We should probably also define nir_op_feq
to be ordered. So we don't have to do anything with them, and !(a ==
b) == (a == b) is guaranteed.
- Define fgt and fle to be ordered, as this is what every backend
already does. No need to add unnecessary NaN checks.
- Disable the comparison simplifications (except for the fne one,
which shouldn't be marked imprecise as it is now) when preserve-nan is
set. I think there are a few other imprecise transforms that also need
to be disabled.
- (optional) Add fgtu and fleu opcodes for unordered comparisons. This
might help backends which can do these in only one instruction. Even
if we don't do this, these can be implemented as not (fle a, b) and
not (fgt a, b) respectively, which is fewer instructions than the
current lowering.
- (optional) Add fequ and fneo opcodes that do unordered equal and
ordered not-equal, respectively. Otherwise they have to be implemented
with explicit NaN checks like now.
On Wed, Dec 5, 2018 at 4:56 PM Samuel Iglesias Gonsálvez
<sigles...@igalia.com> wrote:
This reverts commit c4ab1bdcc9710e3c7cc7115d3be9c69b7e7712ef. We need
to check the arguments looking for NaNs, because they can introduce
failures in tests for FOrd*, specially when running
VK_KHR_shader_float_control tests in CTS.
Signed-off-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
---
src/compiler/spirv/vtn_alu.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/compiler/spirv/vtn_alu.c b/src/compiler/spirv/vtn_alu.c
index dc6fedc9129..629b57560ca 100644
--- a/src/compiler/spirv/vtn_alu.c
+++ b/src/compiler/spirv/vtn_alu.c
@@ -535,18 +535,23 @@ vtn_handle_alu(struct vtn_builder *b, SpvOp opcode,
break;
}
- case SpvOpFOrdNotEqual: {
- /* For all the SpvOpFOrd* comparisons apart from NotEqual, the value
- * from the ALU will probably already be false if the operands are not
- * ordered so we don’t need to handle it specially.
- */
+ case SpvOpFOrdEqual:
+ case SpvOpFOrdNotEqual:
+ case SpvOpFOrdLessThan:
+ case SpvOpFOrdGreaterThan:
+ case SpvOpFOrdLessThanEqual:
+ case SpvOpFOrdGreaterThanEqual: {
bool swap;
unsigned src_bit_size = glsl_get_bit_size(vtn_src[0]->type);
unsigned dst_bit_size = glsl_get_bit_size(type);
nir_op op = vtn_nir_alu_op_for_spirv_opcode(b, opcode, &swap,
src_bit_size, dst_bit_size);
- assert(!swap);
+ if (swap) {
+ nir_ssa_def *tmp = src[0];
+ src[0] = src[1];
+ src[1] = tmp;
+ }
val->ssa->def =
nir_iand(&b->nb,
--
2.19.1
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
