From 59c307a3f33c5d3883658dfe2a1ab6d4e950a699 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 24 Apr 2025 09:48:12 +0200 Subject: [PATCH] pan/bi: Fix and improve the !abs && !coarse case in bi_emit_derivative() The lane source passed to CLPER must be wrap-invariant, and we currently violate this constraint in the !abs && !coarse case. So let's rework the code to use a XOR modifier on the current lane instead, and adjust the sign of the (righ - left) operation afterwards. Signed-off-by: Boris Brezillon Reviewed-by: Lars-Ivar Hesselberg Simonsen Part-of: --- src/panfrost/compiler/bifrost_compile.c | 48 ++++++++++++++++--------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/src/panfrost/compiler/bifrost_compile.c b/src/panfrost/compiler/bifrost_compile.c index b60e81a9bcf..9609b653603 100644 --- a/src/panfrost/compiler/bifrost_compile.c +++ b/src/panfrost/compiler/bifrost_compile.c @@ -1881,33 +1881,49 @@ static void bi_emit_derivative(bi_builder *b, bi_index dst, nir_intrinsic_instr *instr, unsigned axis, bool coarse) { + assert(axis == 1 || axis == 2); + bi_index left, right; bi_index s0 = bi_src_index(&instr->src[0]); + unsigned sz = instr->def.bit_size; /* If all uses are fabs, the sign of the derivative doesn't matter. This is * inherently based on fine derivatives so we can't do it for coarse. */ - if (nir_def_all_uses_ignore_sign_bit(&instr->def) && !coarse) { + if (coarse) { + left = bi_clper(b, s0, bi_imm_u8(0), BI_LANE_OP_NONE); + right = bi_clper(b, s0, bi_imm_u8(axis), BI_LANE_OP_NONE); + } else { left = s0; right = bi_clper(b, s0, bi_imm_u8(axis), BI_LANE_OP_XOR); - } else { - bi_index lane1, lane2; - if (coarse) { - lane1 = bi_imm_u32(0); - lane2 = bi_imm_u32(axis); - } else { - lane1 = bi_lshift_and_i32(b, bi_fau(BIR_FAU_LANE_ID, false), - bi_imm_u32(0x3 & ~axis), bi_imm_u8(0)); - - lane2 = bi_iadd_u32(b, lane1, bi_imm_u32(axis), false); - } - - left = bi_clper(b, s0, bi_byte(lane1, 0), BI_LANE_OP_NONE); - right = bi_clper(b, s0, bi_byte(lane2, 0), BI_LANE_OP_NONE); } - bi_fadd_to(b, sz, dst, right, bi_neg(left)); + if (!coarse && !nir_def_all_uses_ignore_sign_bit(&instr->def)) { + /* If the user cares about the sign, we need to take into account the fact + * left/right (or top/bottom) might be inverted. Instead of using a couple + * CSEL, we just invert the sign bit with + * + * sign_bit = XOR(sign_bit, axis_bit(lane_id)). + */ + bi_index res = bi_fadd(b, sz, right, bi_neg(left)); + bi_index lane = bi_fau(BIR_FAU_LANE_ID, false); + bi_index lane_shift = bi_imm_u8(sz - ffs(axis)); + + /* Clear the low bit before the shift if this is the Y-axis we want. + * We skip it on the X-axis, because the lshift-by-31 will get us a + * clean mask. */ + if (axis == 2) + lane = bi_lshift_and_i32(b, lane, bi_imm_u32(2), bi_imm_u8(0)); + + if (sz == 16) + lane = bi_half(lane, false); + + /* And here comes the final XOR on the sign bit. */ + bi_lshift_xor_to(b, sz, dst, lane, res, lane_shift); + } else { + bi_fadd_to(b, sz, dst, right, bi_neg(left)); + } } static enum bi_subgroup