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: 47c4b38540 ("i965/fs: Allow CSE to handle MULs with negated arguments.")
(cherry picked from commit 14c65322e8)

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/40092>
This commit is contained in:
Matt Turner 2026-02-10 15:25:49 -05:00 committed by Eric Engestrom
parent 03e6f285e5
commit 2c250e6235
2 changed files with 15 additions and 33 deletions

View file

@ -6374,7 +6374,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

View file

@ -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++) {