From 11c6b6c1022991978d941fb3b82cae57b99ec454 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Wed, 1 May 2024 17:01:24 -0700 Subject: [PATCH] intel/elk: Remove dsign optimization This bit from the comment should have been a big red flag: There are currently zero instances of fsign(double(x))*IMM in shader-db or any test suite, so it is hard to care at this time. The implementation of that path was incorrect. The XOR instructions should be predicated like the OR instruction in the non-multiplication path. As a result, dsign(zero_value) * x will not produce the correct result. Instead of fixing this code that is never exercised by anything, replace it with the simple lowering in NIR. Ironically, the vec4 implementation is correct. The odds of encountering an application that is performace limited by dsign performance in vertex processing stages on Ivy Bridge or Haswell is infinitesimal. No shader-db changes on any Intel platform. v2: Delete 's' in emit_fsign as it is now unused. Reviewed-by: Kenneth Graunke [v1] Part-of: --- src/intel/compiler/elk/elk_compiler.c | 1 + src/intel/compiler/elk/elk_fs_nir.cpp | 42 +------------------------ src/intel/compiler/elk/elk_vec4_nir.cpp | 32 +------------------ 3 files changed, 3 insertions(+), 72 deletions(-) diff --git a/src/intel/compiler/elk/elk_compiler.c b/src/intel/compiler/elk/elk_compiler.c index ac2fca18cbf..338ddfa3809 100644 --- a/src/intel/compiler/elk/elk_compiler.c +++ b/src/intel/compiler/elk/elk_compiler.c @@ -68,6 +68,7 @@ elk_compiler_create(void *mem_ctx, const struct intel_device_info *devinfo) nir_lower_drcp | nir_lower_dsqrt | nir_lower_drsq | + nir_lower_dsign | nir_lower_dtrunc | nir_lower_dfloor | nir_lower_dceil | diff --git a/src/intel/compiler/elk/elk_fs_nir.cpp b/src/intel/compiler/elk/elk_fs_nir.cpp index 220a8d0303a..5bb1a3b1684 100644 --- a/src/intel/compiler/elk/elk_fs_nir.cpp +++ b/src/intel/compiler/elk/elk_fs_nir.cpp @@ -781,7 +781,6 @@ static void emit_fsign(nir_to_elk_state &ntb, const fs_builder &bld, const nir_alu_instr *instr, elk_fs_reg result, elk_fs_reg *op, unsigned fsign_src) { - elk_fs_visitor &s = ntb.s; const intel_device_info *devinfo = ntb.devinfo; elk_fs_inst *inst; @@ -863,46 +862,7 @@ emit_fsign(nir_to_elk_state &ntb, const fs_builder &bld, const nir_alu_instr *in inst->predicate = ELK_PREDICATE_NORMAL; } else { - /* For doubles we do the same but we need to consider: - * - * - 2-src instructions can't operate with 64-bit immediates - * - The sign is encoded in the high 32-bit of each DF - * - We need to produce a DF result. - */ - - elk_fs_reg zero = s.vgrf(glsl_double_type()); - bld.MOV(zero, elk_setup_imm_df(bld, 0.0)); - bld.CMP(bld.null_reg_df(), op[0], zero, ELK_CONDITIONAL_NZ); - - bld.MOV(result, zero); - - elk_fs_reg r = subscript(result, ELK_REGISTER_TYPE_UD, 1); - bld.AND(r, subscript(op[0], ELK_REGISTER_TYPE_UD, 1), - elk_imm_ud(0x80000000u)); - - if (instr->op == nir_op_fsign) { - set_predicate(ELK_PREDICATE_NORMAL, - bld.OR(r, r, elk_imm_ud(0x3ff00000u))); - } else { - if (devinfo->has_64bit_int) { - /* This could be done better in some cases. If the scale is an - * immediate with the low 32-bits all 0, emitting a separate XOR and - * OR would allow an algebraic optimization to remove the OR. There - * are currently zero instances of fsign(double(x))*IMM in shader-db - * or any test suite, so it is hard to care at this time. - */ - elk_fs_reg result_int64 = retype(result, ELK_REGISTER_TYPE_UQ); - inst = bld.XOR(result_int64, result_int64, - retype(op[1], ELK_REGISTER_TYPE_UQ)); - } else { - elk_fs_reg result_int64 = retype(result, ELK_REGISTER_TYPE_UQ); - bld.MOV(subscript(result_int64, ELK_REGISTER_TYPE_UD, 0), - subscript(op[1], ELK_REGISTER_TYPE_UD, 0)); - bld.XOR(subscript(result_int64, ELK_REGISTER_TYPE_UD, 1), - subscript(result_int64, ELK_REGISTER_TYPE_UD, 1), - subscript(op[1], ELK_REGISTER_TYPE_UD, 1)); - } - } + unreachable("Should have been lowered by nir_opt_algebraic."); } } diff --git a/src/intel/compiler/elk/elk_vec4_nir.cpp b/src/intel/compiler/elk/elk_vec4_nir.cpp index 6415afc3bb7..f12b76afa36 100644 --- a/src/intel/compiler/elk/elk_vec4_nir.cpp +++ b/src/intel/compiler/elk/elk_vec4_nir.cpp @@ -1699,37 +1699,7 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr) inst->predicate = ELK_PREDICATE_NORMAL; dst.type = ELK_REGISTER_TYPE_F; } else { - /* For doubles we do the same but we need to consider: - * - * - We use a MOV with conditional_mod instead of a CMP so that we can - * skip loading a 0.0 immediate. We use a source modifier on the - * source of the MOV so that we flush denormalized values to 0. - * Since we want to compare against 0, this won't alter the result. - * - We need to extract the high 32-bit of each DF where the sign - * is stored. - * - We need to produce a DF result. - */ - - /* Check for zero */ - src_reg value = op[0]; - value.abs = true; - inst = emit(MOV(dst_null_df(), value)); - inst->conditional_mod = ELK_CONDITIONAL_NZ; - - /* AND each high 32-bit channel with 0x80000000u */ - dst_reg tmp = dst_reg(this, glsl_uvec4_type()); - emit(ELK_VEC4_OPCODE_PICK_HIGH_32BIT, tmp, op[0]); - emit(AND(tmp, src_reg(tmp), elk_imm_ud(0x80000000u))); - - /* Add 1.0 to each channel, predicated to skip the cases where the - * channel's value was 0 - */ - inst = emit(OR(tmp, src_reg(tmp), elk_imm_ud(0x3f800000u))); - inst->predicate = ELK_PREDICATE_NORMAL; - - /* Now convert the result from float to double */ - emit_conversion_to_double(dst, retype(src_reg(tmp), - ELK_REGISTER_TYPE_F)); + unreachable("Should have been lowered by nir_opt_algebraic."); } break;