<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Thu, Feb 15, 2018 at 8:54 AM, Connor Abbott <<a href="mailto:cwabbott0@gmail.com" target="_blank">cwabbott0@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, Feb 14, 2018 at 11:53 PM, Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com">tarceri@itsqueeze.com</a>> wrote: > > > On 15/02/18 04:39, Marek Olšák wrote: >> >> Reviewed-by: Marek Olšák <<a href="mailto:marek.olsak@amd.com">marek.olsak@amd.com</a>> >> >> Marek >> >> On Wed, Feb 14, 2018 at 7:29 AM, Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com">tarceri@itsqueeze.com</a>> >> wrote: >>> >>> Fixes glsl-1.30/execution/isinf-and-isnan* piglit tests for >>> radeonsi and should fix SPIRV errors when LLVM optimises away >>> the workarounds in vtn_handle_alu() for handling ordered >>> comparisons. >>> >>> Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=104905" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=104905</a> >>> --- >>> src/amd/common/ac_nir_to_llvm.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/src/amd/common/ac_nir_to_llvm.c >>> b/src/amd/common/ac_nir_to_llvm.c >>> index a0c5680205..e81f86bb08 100644 >>> --- a/src/amd/common/ac_nir_to_llvm.c >>> +++ b/src/amd/common/ac_nir_to_llvm.c >>> @@ -1792,16 +1792,16 @@ static void visit_alu(struct ac_nir_context *ctx, >>> const nir_alu_instr *instr) >>> result = emit_int_cmp(&ctx->ac, LLVMIntUGE, src[0], >>> src[1]); >>> break; >>> case nir_op_feq: >>> - result = emit_float_cmp(&ctx->ac, LLVMRealUEQ, src[0], >>> src[1]); >>> + result = emit_float_cmp(&ctx->ac, LLVMRealOEQ, src[0], >>> src[1]); >>> break; >>> case nir_op_fne: >>> - result = emit_float_cmp(&ctx->ac, LLVMRealUNE, src[0], >>> src[1]); >>> + result = emit_float_cmp(&ctx->ac, LLVMRealONE, src[0], >>> src[1]); > > > It seems we need to leave this one as is to avoid regressions. This is also > what radeonsi does. </div></div>So, the thing you have to understand is that in LLVM unordered comparisons are precisely the inverse of the ordered comparisons. That is, (a <=(ordered) b) == !(a >(unordered b), (a ==(ordered) b) == !(a !=(unordered) b), and so on. C defines that all comparsions are ordered except !=, so that (a == b) == !(a != b) always holds true. Most hardware follows this convention -- offhand, x86 SSE is the only ISA I know of with separate ordered and unordered comparisons, and LLVM appears to have copied the distinction from them, but no one else has both. I'm not even sure if it's in the IEEE spec. GLSL follows the C convention, so glsl_to_nir just uses nir_op_fne to mean unordered not-equal. spirv_to_nir generates some extra instructions, which then get stripped away later... sigh. I think the right way to untangle this mess is to define that the NIR opcodes should always match the C convention. The separate ordered and unordered opcodes are unnecesary, since one is just the logical negation of the other, and LLVM was a little overzealous -- I'm sure they would get rid of the distinction if they had the chance -- and then they were blindly copied to SPIR-V. spirv_to_nir should just negate the result if necessary rather than emitting the extra code to handle NaN, and ac should use ordered except for not-equals. </blockquote><div> </div><div>I think we should also use ordered for not-equal. Otherwise we have no way to contruct an unordered equal or ordered not-equal using the not-trick. I think that would be more important than trying to keep it in sync with C? </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5"> > > >>> break; >>> case nir_op_flt: >>> - result = emit_float_cmp(&ctx->ac, LLVMRealULT, src[0], >>> src[1]); >>> + result = emit_float_cmp(&ctx->ac, LLVMRealOLT, src[0], >>> src[1]); >>> break; >>> case nir_op_fge: >>> - result = emit_float_cmp(&ctx->ac, LLVMRealUGE, src[0], >>> src[1]); >>> + result = emit_float_cmp(&ctx->ac, LLVMRealOGE, src[0], >>> src[1]); >>> break; >>> case nir_op_ufeq: >>> result = emit_float_cmp(&ctx->ac, LLVMRealUEQ, src[0], >>> src[1]); >>> -- >>> 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> > > _______________________________________________ > 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> _______________________________________________ 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> </div></div></blockquote></div> </div></div>