From 033405cd4bd9ee93755eccdca82d56d6f06ee1af Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Wed, 12 May 2021 12:33:09 -0700 Subject: [PATCH] intel/brw: Combine constants and constant propagation for CSEL No shader-db or fossil-db changes on any Intel platform. This ends up begin helpful in "intel/brw: Use range analysis to optimize fsign." v2: Add integer CSEL support v3: Massive simplification (-20 lines!) of constant propagation logic. Suggested by Ken. Add missing CSEL case in supports_src_as_imm. Noticed by Ken. v4: While MAD can mix F and HF sources on some platforms, CSEL cannot. Found by skqp on TGL. Reviewed-by: Kenneth Graunke [v3] Part-of: --- .../compiler/brw_fs_combine_constants.cpp | 12 ++++++++++ .../compiler/brw_fs_copy_propagation.cpp | 23 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/intel/compiler/brw_fs_combine_constants.cpp b/src/intel/compiler/brw_fs_combine_constants.cpp index ab57190d915..bb2ecfb572c 100644 --- a/src/intel/compiler/brw_fs_combine_constants.cpp +++ b/src/intel/compiler/brw_fs_combine_constants.cpp @@ -999,6 +999,10 @@ supports_src_as_imm(const struct intel_device_info *devinfo, const fs_inst *inst /* ADD3 only exists on Gfx12.5+. */ return true; + case BRW_OPCODE_CSEL: + /* While MAD can mix F and HF sources on some platforms, CSEL cannot. */ + return inst->src[0].type != BRW_TYPE_F; + case BRW_OPCODE_MAD: /* Integer types can always mix sizes. Floating point types can mix * sizes on Gfx12. On Gfx12.5, floating point sources must all be HF or @@ -1301,7 +1305,15 @@ brw_fs_opt_combine_constants(fs_visitor &s) } break; + /* FINISHME: CSEL handling could be better. For some cases, src[0] and + * src[1] can be commutative (e.g., any integer comparison). In those + * cases when src[1] is IMM, the sources could be exchanged. In + * addition, when both sources are IMM that could be represented as + * 16-bits, it would be better to add both sources with + * allow_one_constant=true as is done for SEL. + */ case BRW_OPCODE_ADD3: + case BRW_OPCODE_CSEL: case BRW_OPCODE_MAD: { for (int i = 0; i < inst->sources; i++) { if (inst->src[i].file != IMM) diff --git a/src/intel/compiler/brw_fs_copy_propagation.cpp b/src/intel/compiler/brw_fs_copy_propagation.cpp index a3a534a4e08..44f7c4fd979 100644 --- a/src/intel/compiler/brw_fs_copy_propagation.cpp +++ b/src/intel/compiler/brw_fs_copy_propagation.cpp @@ -1168,6 +1168,29 @@ try_constant_propagate(const brw_compiler *compiler, fs_inst *inst, } break; + case BRW_OPCODE_CSEL: + assert(inst->conditional_mod != BRW_CONDITIONAL_NONE); + + if (arg == 0 && + inst->src[1].file != IMM && + (!brw_type_is_float(inst->src[1].type) || + inst->conditional_mod == BRW_CONDITIONAL_NZ || + inst->conditional_mod == BRW_CONDITIONAL_Z)) { + /* Only EQ and NE are commutative due to NaN issues. */ + inst->src[0] = inst->src[1]; + inst->src[1] = val; + inst->conditional_mod = brw_negate_cmod(inst->conditional_mod); + } else { + /* While CSEL is a 3-source instruction, the last source should never + * be a constant. We'll support that, but should it ever happen, we + * should add support to the constant folding pass. + */ + inst->src[arg] = val; + } + + progress = true; + break; + case FS_OPCODE_FB_WRITE_LOGICAL: /* The stencil and omask sources of FS_OPCODE_FB_WRITE_LOGICAL are * bit-cast using a strided region so they cannot be immediates.