<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Mar 21, 2018 at 12:34 PM, Neil Roberts <<a href="mailto:nroberts@igalia.com" target="_blank">nroberts@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The SPIR-V spec doesn’t specify a size requirement for these and the equivalent functions in the GLSL spec have explicit alternatives for doubles. Refract is a little bit more complicated due to the fact that the final argument is always supposed to be a scalar 32- or 16- bit float regardless of the other operands. However in practice it seems there is a bug in glslang that makes it convert the argument to 64-bit if you actually try to pass it a 32-bit value while the other arguments are 64-bit. This adds an optional conversion of the final argument in order to support any type. These have been tested against the automatically generated tests of glsl-4.00/execution/built-in-functions using the ARB_gl_spirv branch which tests it with quite a large range of combinations. The issue with glslang has been filed here: <a href="https://github.com/KhronosGroup/glslang/issues/1279" rel="noreferrer" target="_blank">https://github.com/KhronosGroup/glslang/issues/1279</a> v2: Convert the eta operand of Refract from any size in order to make it eventually cope with 16-bit floats. --- src/compiler/spirv/vtn_glsl450.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/compiler/spirv/vtn_glsl450.c b/src/compiler/spirv/vtn_glsl450.c index 50783fbfb4d..0cabedf741d 100644 --- a/src/compiler/spirv/vtn_glsl450.c +++ b/src/compiler/spirv/vtn_glsl450.c @@ -628,14 +628,14 @@ handle_glsl450_alu(struct vtn_builder *b, enum GLSLstd450 entrypoint, case GLSLstd450FaceForward: val->ssa->def = nir_bcsel(nb, nir_flt(nb, nir_fdot(nb, src[2], src[1]), - nir_imm_float(nb, 0.0)), + NIR_IMM_FP(nb, 0.0)), src[0], nir_fneg(nb, src[0])); return; case GLSLstd450Reflect: /* I - 2 * dot(N, I) * N */ val->ssa->def = - nir_fsub(nb, src[0], nir_fmul(nb, nir_imm_float(nb, 2.0), + nir_fsub(nb, src[0], nir_fmul(nb, NIR_IMM_FP(nb, 2.0), nir_fmul(nb, nir_fdot(nb, src[0], src[1]), src[1]))); return; @@ -645,8 +645,22 @@ handle_glsl450_alu(struct vtn_builder *b, enum GLSLstd450 entrypoint, nir_ssa_def *N = src[1]; nir_ssa_def *eta = src[2]; nir_ssa_def *n_dot_i = nir_fdot(nb, N, I); - nir_ssa_def *one = nir_imm_float(nb, 1.0); - nir_ssa_def *zero = nir_imm_float(nb, 0.0); + nir_ssa_def *one = NIR_IMM_FP(nb, 1.0); + nir_ssa_def *zero = NIR_IMM_FP(nb, 0.0); + /* According to the SPIR-V and GLSL specs, eta is always a float + * regardless of the type of the other operands. However in practice it + * seems that if you try to pass it a float then glslang will just + * promote it to a double and generate invalid SPIR-V. In order to + * support a hypothetical fixed version of glslang we’ll promote eta to + * double if the other operands are double also. + */ </blockquote><div> </div><div>That's a bit ugly. Seems like the right thing to do though </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + if (I->bit_size != eta->bit_size) { + nir_op conversion_op = + nir_type_conversion_op(nir_type_float | eta->bit_size, + nir_type_float | I->bit_size, + nir_rounding_mode_undef); + eta = nir_build_alu(nb, conversion_op, eta, NULL, NULL, NULL); + } /* k = 1.0 - eta * eta * (1.0 - dot(N, I) * dot(N, I)) */ nir_ssa_def *k = nir_fsub(nb, one, nir_fmul(nb, eta, nir_fmul(nb, eta, -- 2.14.3 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>