[Mesa-dev] [PATCH] spirv: Don’t check for NaN for most OpFOrd* comparisons
Bas Nieuwenhuizen
bas at basnieuwenhuizen.nl
Tue Apr 24 17:24:07 UTC 2018
Reviewed-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
On Tue, Apr 24, 2018 at 7:55 AM, Neil Roberts <nroberts at igalia.com> wrote:
> For all of the OpFOrd* comparisons except OpFOrdNotEqual the hardware
> should probably already return false if one of the operands is NaN so
> we don’t need to have an explicit check for it. This seems to at least
> work on Intel hardware. This should reduce the number of instructions
> generated for the most common comparisons.
>
> For what it’s worth, the original code to handle this was added in
> e062eb6415de3a. The commit message for that says that it was to fix
> some CTS tests for OpFUnord* opcodes. Even if the hardware doesn’t
> handle NaNs this patch shouldn’t affect those tests. At any rate they
> have since been moved out of the mustpass list. Incidentally those
> tests fail on the nvidia proprietary driver so it doesn’t seem like
> handling NaNs correctly is a priority.
> ---
>
> I made a VkRunner test case for all of the OpFOrd* and OpFUnord*
> opcodes with and without NaNs on the test branch. It can be run like
> this:
>
> git clone -b tests https://github.com/Igalia/vkrunner.git
> cd vkrunner
> ./autogen.sh && make -j8
> ./src/vkrunner examples/unordered-comparison.shader_test
>
> src/compiler/spirv/vtn_alu.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/src/compiler/spirv/vtn_alu.c b/src/compiler/spirv/vtn_alu.c
> index 71e743cdd1e..3134849ba90 100644
> --- a/src/compiler/spirv/vtn_alu.c
> +++ b/src/compiler/spirv/vtn_alu.c
> @@ -597,23 +597,18 @@ vtn_handle_alu(struct vtn_builder *b, SpvOp opcode,
> break;
> }
>
> - case SpvOpFOrdEqual:
> - case SpvOpFOrdNotEqual:
> - case SpvOpFOrdLessThan:
> - case SpvOpFOrdGreaterThan:
> - case SpvOpFOrdLessThanEqual:
> - case SpvOpFOrdGreaterThanEqual: {
> + 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.
> + */
> 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);
>
> - if (swap) {
> - nir_ssa_def *tmp = src[0];
> - src[0] = src[1];
> - src[1] = tmp;
> - }
> + assert(!swap);
>
> val->ssa->def =
> nir_iand(&b->nb,
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list