From 19acd105c837114636ad55db85351a343e488609 Mon Sep 17 00:00:00 2001 From: Matt Turner Date: Tue, 10 Feb 2026 15:25:49 -0500 Subject: [PATCH] elk/cse: use copies in `operands_match` instead of in-place modification `operands_match` was modifying instruction source operands in-place (through the `elk_fs_reg *src` pointer member) and relying on a save/restore pattern to undo the modifications. Work on local copies instead, which is simpler and avoids mutating shared state in a comparison function. Fixes: 47c4b385407 ("i965/fs: Allow CSE to handle MULs with negated arguments.") (cherry picked from commit 14c65322e8504c498115fe6c11b095d555e03f2d) Part-of: --- .pick_status.json | 2 +- src/intel/compiler/elk/elk_fs_cse.cpp | 46 ++++++++------------------- 2 files changed, 15 insertions(+), 33 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 936dbe8e345..919653d026d 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -664,7 +664,7 @@ "description": "elk/cse: use copies in `operands_match` instead of in-place modification", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "47c4b3854076adfe5a27b537f36262ac4ec4530d", "notes": null diff --git a/src/intel/compiler/elk/elk_fs_cse.cpp b/src/intel/compiler/elk/elk_fs_cse.cpp index e5399bf365f..1a65871bb5e 100644 --- a/src/intel/compiler/elk/elk_fs_cse.cpp +++ b/src/intel/compiler/elk/elk_fs_cse.cpp @@ -117,8 +117,8 @@ is_expression(const elk_fs_visitor *v, const elk_fs_inst *const inst) static bool operands_match(const elk_fs_inst *a, const elk_fs_inst *b, bool *negate) { - elk_fs_reg *xs = a->src; - elk_fs_reg *ys = b->src; + const elk_fs_reg *xs = a->src; + const elk_fs_reg *ys = b->src; if (a->opcode == ELK_OPCODE_MAD) { return xs[0].equals(ys[0]) && @@ -131,41 +131,23 @@ operands_match(const elk_fs_inst *a, const elk_fs_inst *b, bool *negate) bool ys0_negate = ys[0].negate; bool ys1_negate = ys[1].file == IMM ? ys[1].f < 0.0f : ys[1].negate; - float xs1_imm = 0, ys1_imm = 0; + /* Work on copies to avoid modifying the original instructions. */ + elk_fs_reg src[4] = { xs[0], xs[1], ys[0], ys[1] }; - xs[0].negate = false; - xs[1].negate = false; - ys[0].negate = false; - ys[1].negate = false; - /* Only access .f when the register is an immediate. When it is not, - * .f aliases the struct containing nr/swizzle/etc, so fabsf() could - * corrupt the register number by clearing bit 31. - */ - if (xs[1].file == IMM) { - xs1_imm = xs[1].f; - xs[1].f = fabsf(xs1_imm); - } - if (ys[1].file == IMM) { - ys1_imm = ys[1].f; - ys[1].f = fabsf(ys1_imm); - } - - bool ret = (xs[0].equals(ys[0]) && xs[1].equals(ys[1])) || - (xs[1].equals(ys[0]) && xs[0].equals(ys[1])); - - xs[0].negate = xs0_negate; - xs[1].negate = xs[1].file == IMM ? false : xs1_negate; - ys[0].negate = ys0_negate; - ys[1].negate = ys[1].file == IMM ? false : ys1_negate; - if (xs[1].file == IMM) - xs[1].f = xs1_imm; - if (ys[1].file == IMM) - ys[1].f = ys1_imm; + src[0].negate = false; + src[1].negate = false; + src[2].negate = false; + src[3].negate = false; + if (src[1].file == IMM) + src[1].f = fabsf(src[1].f); + if (src[3].file == IMM) + src[3].f = fabsf(src[3].f); *negate = (xs0_negate != xs1_negate) != (ys0_negate != ys1_negate); if (*negate && (a->saturate || b->saturate)) return false; - return ret; + return (src[0].equals(src[2]) && src[1].equals(src[3])) || + (src[1].equals(src[2]) && src[0].equals(src[3])); } else if (!a->is_commutative()) { bool match = true; for (int i = 0; i < a->sources; i++) {