From 55bdae03ccf0a5f49df2eb570a326ba28c573964 Mon Sep 17 00:00:00 2001 From: Paulo Zanoni Date: Fri, 14 Feb 2025 17:21:24 -0800 Subject: [PATCH] brw: don't always set cond_modifier on parsed assembly instructions For the instructions we parse with brw_gram.y, don't unconditionally call brw_eu_inst_set_cond_modifier(). Do it like we do in brw_generator::generate_code() and only call it if we have a cond_modifier to set. Why? Because for ONE_SRC instructions, CondCtrl (bits 95:92) only exists if Src.IsImm is false. If Src.Imm is true, then bits 95:64 are actually Src0.ImmValue[63:32]. If we unconditionally call brw_eu_inst_set_cond_modifier(), we'll end up zeroing bits 95:92 for ONE_SRC instructions with 64bit immediates. See BSpec page Structure_EU_INSTRUCTION_BASIC_ONE_SRC (56880). This issue can be reproduced with src/intel/executor if you try to have the following instruction: mov(16) g10<1>Q 0xfedcba9876543210:Q { align1 WE_all 1H }; our parser will end up zeroing the top bits, so the value of the immediate will be 0x0edcba9876543210. Reviewed-by: Caio Oliveira Signed-off-by: Paulo Zanoni Part-of: --- src/intel/compiler/brw_gram.y | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/intel/compiler/brw_gram.y b/src/intel/compiler/brw_gram.y index fa4c44f8a96..b6cdd2f1a61 100644 --- a/src/intel/compiler/brw_gram.y +++ b/src/intel/compiler/brw_gram.y @@ -674,8 +674,11 @@ unaryinstruction: i965_asm_unary_instruction($2, p, $6, $7); brw_pop_insn_state(p); i965_asm_set_instruction_options(p, $8); - brw_eu_inst_set_cond_modifier(p->devinfo, brw_last_inst, - $4.cond_modifier); + if ($4.cond_modifier) { + brw_eu_inst_set_cond_modifier(p->devinfo, + brw_last_inst, + $4.cond_modifier); + } if (!brw_eu_inst_flag_reg_nr(p->devinfo, brw_last_inst)) { brw_eu_inst_set_flag_reg_nr(p->devinfo, @@ -719,8 +722,11 @@ binaryinstruction: brw_set_default_access_mode(p, $9.access_mode); i965_asm_binary_instruction($2, p, $6, $7, $8); i965_asm_set_instruction_options(p, $9); - brw_eu_inst_set_cond_modifier(p->devinfo, brw_last_inst, - $4.cond_modifier); + if ($4.cond_modifier) { + brw_eu_inst_set_cond_modifier(p->devinfo, + brw_last_inst, + $4.cond_modifier); + } if (!brw_eu_inst_flag_reg_nr(p->devinfo, brw_last_inst)) { brw_eu_inst_set_flag_reg_nr(p->devinfo, brw_last_inst, @@ -762,8 +768,11 @@ binaryaccinstruction: i965_asm_binary_instruction($2, p, $6, $7, $8); brw_pop_insn_state(p); i965_asm_set_instruction_options(p, $9); - brw_eu_inst_set_cond_modifier(p->devinfo, brw_last_inst, - $4.cond_modifier); + if ($4.cond_modifier) { + brw_eu_inst_set_cond_modifier(p->devinfo, + brw_last_inst, + $4.cond_modifier); + } if (!brw_eu_inst_flag_reg_nr(p->devinfo, brw_last_inst)) { brw_eu_inst_set_flag_reg_nr(p->devinfo, @@ -842,8 +851,11 @@ ternaryinstruction: i965_asm_ternary_instruction($2, p, $6, $7, $8, $9); brw_pop_insn_state(p); i965_asm_set_instruction_options(p, $10); - brw_eu_inst_set_cond_modifier(p->devinfo, brw_last_inst, - $4.cond_modifier); + if ($4.cond_modifier) { + brw_eu_inst_set_cond_modifier(p->devinfo, + brw_last_inst, + $4.cond_modifier); + } if (p->devinfo->ver < 12) { brw_eu_inst_set_3src_a16_flag_reg_nr(p->devinfo, brw_last_inst,