From 148473eae4cd202c83fc49f462f08bff14506bc8 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Tue, 25 Apr 2023 09:04:25 +0200 Subject: [PATCH] broadcom/compiler: fix incorrect ALU checks MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit We had a bunch of cases where we would check ALU parameters without first checking if the ALU op was valid. Reviewed-by: Alejandro Piñeiro Part-of: --- src/broadcom/compiler/qpu_schedule.c | 30 ++++++++++++++---------------- src/broadcom/qpu/qpu_instr.c | 30 ++++++++++++++++++++---------- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/broadcom/compiler/qpu_schedule.c b/src/broadcom/compiler/qpu_schedule.c index c5742db..9061d2e 100644 --- a/src/broadcom/compiler/qpu_schedule.c +++ b/src/broadcom/compiler/qpu_schedule.c @@ -136,12 +136,14 @@ qpu_inst_is_tlb(const struct v3d_qpu_instr *inst) if (inst->type != V3D_QPU_INSTR_TYPE_ALU) return false; - if (inst->alu.add.magic_write && + if (inst->alu.add.op != V3D_QPU_A_NOP && + inst->alu.add.magic_write && (inst->alu.add.waddr == V3D_QPU_WADDR_TLB || inst->alu.add.waddr == V3D_QPU_WADDR_TLBU)) return true; - if (inst->alu.mul.magic_write && + if (inst->alu.mul.op != V3D_QPU_M_NOP && + inst->alu.mul.magic_write && (inst->alu.mul.waddr == V3D_QPU_WADDR_TLB || inst->alu.mul.waddr == V3D_QPU_WADDR_TLBU)) return true; @@ -1458,14 +1460,16 @@ instruction_latency(const struct v3d_device_info *devinfo, after_inst->type != V3D_QPU_INSTR_TYPE_ALU) return latency; - if (before_inst->alu.add.magic_write) { + if (before_inst->alu.add.op != V3D_QPU_A_NOP && + before_inst->alu.add.magic_write) { latency = MAX2(latency, magic_waddr_latency(devinfo, before_inst->alu.add.waddr, after_inst)); } - if (before_inst->alu.mul.magic_write) { + if (before_inst->alu.mul.op != V3D_QPU_M_NOP && + before_inst->alu.mul.magic_write) { latency = MAX2(latency, magic_waddr_latency(devinfo, before_inst->alu.mul.waddr, @@ -1588,8 +1592,10 @@ qpu_inst_valid_in_thrend_slot(struct v3d_compile *c, return false; /* No writing physical registers at the end. */ - if (!inst->alu.add.magic_write || - !inst->alu.mul.magic_write) { + bool add_is_nop = inst->alu.add.op == V3D_QPU_A_NOP; + bool mul_is_nop = inst->alu.mul.op == V3D_QPU_M_NOP; + if ((!add_is_nop && !inst->alu.add.magic_write) || + (!mul_is_nop && !inst->alu.mul.magic_write)) { return false; } @@ -1604,20 +1610,12 @@ qpu_inst_valid_in_thrend_slot(struct v3d_compile *c, /* RF0-2 might be overwritten during the delay slots by * fragment shader setup. */ - if (inst->raddr_a < 3 && - (inst->alu.add.a == V3D_QPU_MUX_A || - inst->alu.add.b == V3D_QPU_MUX_A || - inst->alu.mul.a == V3D_QPU_MUX_A || - inst->alu.mul.b == V3D_QPU_MUX_A)) { + if (inst->raddr_a < 3 && v3d_qpu_uses_mux(inst, V3D_QPU_MUX_A)) return false; - } if (inst->raddr_b < 3 && !inst->sig.small_imm && - (inst->alu.add.a == V3D_QPU_MUX_B || - inst->alu.add.b == V3D_QPU_MUX_B || - inst->alu.mul.a == V3D_QPU_MUX_B || - inst->alu.mul.b == V3D_QPU_MUX_B)) { + v3d_qpu_uses_mux(inst, V3D_QPU_MUX_B)) { return false; } } diff --git a/src/broadcom/qpu/qpu_instr.c b/src/broadcom/qpu/qpu_instr.c index 1d9c8e6..60dabf7 100644 --- a/src/broadcom/qpu/qpu_instr.c +++ b/src/broadcom/qpu/qpu_instr.c @@ -643,12 +643,14 @@ v3d_qpu_uses_tlb(const struct v3d_qpu_instr *inst) return true; if (inst->type == V3D_QPU_INSTR_TYPE_ALU) { - if (inst->alu.add.magic_write && + if (inst->alu.add.op != V3D_QPU_A_NOP && + inst->alu.add.magic_write && v3d_qpu_magic_waddr_is_tlb(inst->alu.add.waddr)) { return true; } - if (inst->alu.mul.magic_write && + if (inst->alu.mul.op != V3D_QPU_M_NOP && + inst->alu.mul.magic_write && v3d_qpu_magic_waddr_is_tlb(inst->alu.mul.waddr)) { return true; } @@ -710,9 +712,11 @@ v3d_qpu_writes_tmu(const struct v3d_device_info *devinfo, const struct v3d_qpu_instr *inst) { return (inst->type == V3D_QPU_INSTR_TYPE_ALU && - ((inst->alu.add.magic_write && + ((inst->alu.add.op != V3D_QPU_A_NOP && + inst->alu.add.magic_write && v3d_qpu_magic_waddr_is_tmu(devinfo, inst->alu.add.waddr)) || - (inst->alu.mul.magic_write && + (inst->alu.mul.op != V3D_QPU_M_NOP && + inst->alu.mul.magic_write && v3d_qpu_magic_waddr_is_tmu(devinfo, inst->alu.mul.waddr)))); } @@ -748,12 +752,14 @@ v3d_qpu_writes_vpm(const struct v3d_qpu_instr *inst) if (v3d_qpu_add_op_writes_vpm(inst->alu.add.op)) return true; - if (inst->alu.add.magic_write && + if (inst->alu.add.op != V3D_QPU_A_NOP && + inst->alu.add.magic_write && v3d_qpu_magic_waddr_is_vpm(inst->alu.add.waddr)) { return true; } - if (inst->alu.mul.magic_write && + if (inst->alu.mul.op != V3D_QPU_M_NOP && + inst->alu.mul.magic_write && v3d_qpu_magic_waddr_is_vpm(inst->alu.mul.waddr)) { return true; } @@ -819,10 +825,12 @@ qpu_writes_magic_waddr_explicitly(const struct v3d_device_info *devinfo, uint32_t waddr) { if (inst->type == V3D_QPU_INSTR_TYPE_ALU) { - if (inst->alu.add.magic_write && inst->alu.add.waddr == waddr) + if (inst->alu.add.op != V3D_QPU_A_NOP && + inst->alu.add.magic_write && inst->alu.add.waddr == waddr) return true; - if (inst->alu.mul.magic_write && inst->alu.mul.waddr == waddr) + if (inst->alu.mul.op != V3D_QPU_M_NOP && + inst->alu.mul.magic_write && inst->alu.mul.waddr == waddr) return true; } @@ -849,13 +857,15 @@ v3d_qpu_writes_r4(const struct v3d_device_info *devinfo, const struct v3d_qpu_instr *inst) { if (inst->type == V3D_QPU_INSTR_TYPE_ALU) { - if (inst->alu.add.magic_write && + if (inst->alu.add.op != V3D_QPU_A_NOP && + inst->alu.add.magic_write && (inst->alu.add.waddr == V3D_QPU_WADDR_R4 || v3d_qpu_magic_waddr_is_sfu(inst->alu.add.waddr))) { return true; } - if (inst->alu.mul.magic_write && + if (inst->alu.mul.op != V3D_QPU_M_NOP && + inst->alu.mul.magic_write && (inst->alu.mul.waddr == V3D_QPU_WADDR_R4 || v3d_qpu_magic_waddr_is_sfu(inst->alu.mul.waddr))) { return true; -- 2.7.4