From: Rhys Perry Date: Wed, 3 Jun 2020 10:27:55 +0000 (+0100) Subject: aco: validate instructions reading/writing upper halves/bytes X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=d9cfb8ad483ed9ef182c62b5fac55f5e356540a2;p=mesa.git aco: validate instructions reading/writing upper halves/bytes Signed-off-by: Rhys Perry Reviewed-by: Daniel Schürmann Part-of: --- diff --git a/src/amd/compiler/aco_ir.cpp b/src/amd/compiler/aco_ir.cpp new file mode 100644 index 00000000000..f9ee3d78bfa --- /dev/null +++ b/src/amd/compiler/aco_ir.cpp @@ -0,0 +1,74 @@ +/* + * Copyright © 2020 Valve Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ +#include "aco_ir.h" + +namespace aco { + +bool can_use_opsel(chip_class chip, aco_opcode op, int idx, bool high) +{ + /* opsel is only GFX9+ */ + if ((high || idx == -1) && chip < GFX9) + return false; + + switch (op) { + case aco_opcode::v_div_fixup_f16: + case aco_opcode::v_fma_f16: + case aco_opcode::v_mad_f16: + case aco_opcode::v_mad_u16: + case aco_opcode::v_mad_i16: + case aco_opcode::v_med3_f16: + case aco_opcode::v_med3_i16: + case aco_opcode::v_med3_u16: + case aco_opcode::v_min3_f16: + case aco_opcode::v_min3_i16: + case aco_opcode::v_min3_u16: + case aco_opcode::v_max3_f16: + case aco_opcode::v_max3_i16: + case aco_opcode::v_max3_u16: + case aco_opcode::v_max_u16_e64: + case aco_opcode::v_max_i16_e64: + case aco_opcode::v_min_u16_e64: + case aco_opcode::v_min_i16_e64: + case aco_opcode::v_add_i16: + case aco_opcode::v_sub_i16: + case aco_opcode::v_add_u16_e64: + case aco_opcode::v_sub_u16_e64: + case aco_opcode::v_cvt_pknorm_i16_f16: + case aco_opcode::v_cvt_pknorm_u16_f16: + case aco_opcode::v_lshlrev_b16_e64: + case aco_opcode::v_lshrrev_b16_e64: + case aco_opcode::v_ashrrev_i16_e64: + case aco_opcode::v_mul_lo_u16_e64: + return true; + case aco_opcode::v_pack_b32_f16: + return idx != -1; + case aco_opcode::v_mad_u32_u16: + case aco_opcode::v_mad_i32_i16: + return idx >= 0 && idx < 2; + default: + return false; + } +} + +} diff --git a/src/amd/compiler/aco_ir.h b/src/amd/compiler/aco_ir.h index a25efe1e7a1..4e8aa372dff 100644 --- a/src/amd/compiler/aco_ir.h +++ b/src/amd/compiler/aco_ir.h @@ -1223,6 +1223,8 @@ barrier_interaction get_barrier_interaction(const Instruction* instr); bool is_dead(const std::vector& uses, Instruction *instr); +bool can_use_opsel(chip_class chip, aco_opcode op, int idx, bool high); + enum block_kind { /* uniform indicates that leaving this block, * all actives lanes stay active */ @@ -1430,6 +1432,7 @@ public: unsigned workgroup_size; /* if known; otherwise UINT_MAX */ bool xnack_enabled = false; + bool sram_ecc_enabled = false; bool needs_vcc = false; bool needs_flat_scr = false; diff --git a/src/amd/compiler/aco_validate.cpp b/src/amd/compiler/aco_validate.cpp index 65ca2b45065..6d7c0c3e794 100644 --- a/src/amd/compiler/aco_validate.cpp +++ b/src/amd/compiler/aco_validate.cpp @@ -146,6 +146,13 @@ void validate(Program* program, FILE * output) instr->opcode != aco_opcode::v_fmac_f16, "SDWA can't be used with this opcode", instr.get()); } + + for (unsigned i = 0; i < MIN2(instr->operands.size(), 2); i++) { + if (instr->operands[i].regClass().is_subdword()) + check((sdwa->sel[i] & sdwa_asuint) == (sdwa_isra | instr->operands[i].bytes()), "Unexpected SDWA sel for sub-dword operand", instr.get()); + } + if (instr->definitions[0].regClass().is_subdword()) + check((sdwa->dst_sel & sdwa_asuint) == (sdwa_isra | instr->definitions[0].bytes()), "Unexpected SDWA sel for sub-dword definition", instr.get()); } /* check opsel */ @@ -153,6 +160,13 @@ void validate(Program* program, FILE * output) VOP3A_instruction *vop3 = static_cast(instr.get()); check(vop3->opsel == 0 || program->chip_class >= GFX9, "Opsel is only supported on GFX9+", instr.get()); check((vop3->opsel & ~(0x10 | ((1 << instr->operands.size()) - 1))) == 0, "Unused bits in opsel must be zeroed out", instr.get()); + + for (unsigned i = 0; i < instr->operands.size(); i++) { + if (instr->operands[i].regClass().is_subdword()) + check((vop3->opsel & (1 << i)) == 0, "Unexpected opsel for sub-dword operand", instr.get()); + } + if (instr->definitions[0].regClass().is_subdword()) + check((vop3->opsel & (1 << 3)) == 0, "Unexpected opsel for sub-dword definition", instr.get()); } /* check for undefs */ @@ -429,11 +443,132 @@ bool ra_fail(FILE *output, Location loc, Location loc2, const char *fmt, ...) { return true; } -bool instr_can_access_subdword(Program* program, aco_ptr& instr) +bool validate_subdword_operand(chip_class chip, const aco_ptr& instr, unsigned index) { - if (program->chip_class < GFX8) - return false; - return instr->isSDWA() || instr->format == Format::PSEUDO; + Operand op = instr->operands[index]; + unsigned byte = op.physReg().byte(); + + if (instr->format == Format::PSEUDO && chip >= GFX8) + return true; + if (instr->isSDWA() && (static_cast(instr.get())->sel[index] & sdwa_asuint) == (sdwa_isra | op.bytes())) + return true; + if (byte == 2 && can_use_opsel(chip, instr->opcode, index, 1)) + return true; + + switch (instr->opcode) { + case aco_opcode::v_cvt_f32_ubyte1: + if (byte == 1) + return true; + break; + case aco_opcode::v_cvt_f32_ubyte2: + if (byte == 2) + return true; + break; + case aco_opcode::v_cvt_f32_ubyte3: + if (byte == 3) + return true; + break; + case aco_opcode::ds_write_b8_d16_hi: + case aco_opcode::ds_write_b16_d16_hi: + if (byte == 2 && index == 1) + return true; + break; + case aco_opcode::buffer_store_byte_d16_hi: + case aco_opcode::buffer_store_short_d16_hi: + if (byte == 2 && index == 3) + return true; + break; + case aco_opcode::flat_store_byte_d16_hi: + case aco_opcode::flat_store_short_d16_hi: + case aco_opcode::scratch_store_byte_d16_hi: + case aco_opcode::scratch_store_short_d16_hi: + case aco_opcode::global_store_byte_d16_hi: + case aco_opcode::global_store_short_d16_hi: + if (byte == 2 && index == 2) + return true; + default: + break; + } + + return byte == 0; +} + +bool validate_subdword_definition(chip_class chip, const aco_ptr& instr) +{ + Definition def = instr->definitions[0]; + unsigned byte = def.physReg().byte(); + + if (instr->format == Format::PSEUDO && chip >= GFX8) + return true; + if (instr->isSDWA() && static_cast(instr.get())->dst_sel == (sdwa_isra | def.bytes())) + return true; + if (byte == 2 && can_use_opsel(chip, instr->opcode, -1, 1)) + return true; + + switch (instr->opcode) { + case aco_opcode::buffer_load_ubyte_d16_hi: + case aco_opcode::buffer_load_short_d16_hi: + case aco_opcode::flat_load_ubyte_d16_hi: + case aco_opcode::flat_load_short_d16_hi: + case aco_opcode::scratch_load_ubyte_d16_hi: + case aco_opcode::scratch_load_short_d16_hi: + case aco_opcode::global_load_ubyte_d16_hi: + case aco_opcode::global_load_short_d16_hi: + case aco_opcode::ds_read_u8_d16_hi: + case aco_opcode::ds_read_u16_d16_hi: + return byte == 2; + default: + break; + } + + return byte == 0; +} + +unsigned get_subdword_bytes_written(Program *program, const aco_ptr& instr, unsigned index) +{ + chip_class chip = program->chip_class; + Definition def = instr->definitions[index]; + + if (instr->format == Format::PSEUDO) + return chip >= GFX8 ? def.bytes() : def.size() * 4u; + if (instr->isSDWA() && static_cast(instr.get())->dst_sel == (sdwa_isra | def.bytes())) + return def.bytes(); + + switch (instr->opcode) { + case aco_opcode::buffer_load_ubyte_d16: + case aco_opcode::buffer_load_short_d16: + case aco_opcode::flat_load_ubyte_d16: + case aco_opcode::flat_load_short_d16: + case aco_opcode::scratch_load_ubyte_d16: + case aco_opcode::scratch_load_short_d16: + case aco_opcode::global_load_ubyte_d16: + case aco_opcode::global_load_short_d16: + case aco_opcode::ds_read_u8_d16: + case aco_opcode::ds_read_u16_d16: + case aco_opcode::buffer_load_ubyte_d16_hi: + case aco_opcode::buffer_load_short_d16_hi: + case aco_opcode::flat_load_ubyte_d16_hi: + case aco_opcode::flat_load_short_d16_hi: + case aco_opcode::scratch_load_ubyte_d16_hi: + case aco_opcode::scratch_load_short_d16_hi: + case aco_opcode::global_load_ubyte_d16_hi: + case aco_opcode::global_load_short_d16_hi: + case aco_opcode::ds_read_u8_d16_hi: + case aco_opcode::ds_read_u16_d16_hi: + return program->sram_ecc_enabled ? 4 : 2; + case aco_opcode::v_mad_f16: + case aco_opcode::v_mad_u16: + case aco_opcode::v_mad_i16: + case aco_opcode::v_fma_f16: + case aco_opcode::v_div_fixup_f16: + case aco_opcode::v_interp_p2_f16: + if (chip >= GFX9) + return 2; + default: + break; + } + + return chip >= GFX10 ? def.bytes() : 4; } } /* end namespace */ @@ -474,8 +609,8 @@ bool validate_ra(Program *program, const struct radv_nir_compiler_options *optio err |= ra_fail(output, loc, assignments.at(op.tempId()).firstloc, "Operand %d has an out-of-bounds register assignment", i); if (op.physReg() == vcc && !program->needs_vcc) err |= ra_fail(output, loc, Location(), "Operand %d fixed to vcc but needs_vcc=false", i); - if (!instr_can_access_subdword(program, instr) && op.regClass().is_subdword() && op.physReg().byte()) - err |= ra_fail(output, loc, assignments.at(op.tempId()).firstloc, "Operand %d must be aligned to a full register", i); + if (op.regClass().is_subdword() && !validate_subdword_operand(program->chip_class, instr, i)) + err |= ra_fail(output, loc, Location(), "Operand %d not aligned correctly", i); if (!assignments[op.tempId()].firstloc.block) assignments[op.tempId()].firstloc = loc; if (!assignments[op.tempId()].defloc.block) @@ -495,8 +630,8 @@ bool validate_ra(Program *program, const struct radv_nir_compiler_options *optio err |= ra_fail(output, loc, assignments.at(def.tempId()).firstloc, "Definition %d has an out-of-bounds register assignment", i); if (def.physReg() == vcc && !program->needs_vcc) err |= ra_fail(output, loc, Location(), "Definition %d fixed to vcc but needs_vcc=false", i); - if (!instr_can_access_subdword(program, instr) && def.regClass().is_subdword() && def.physReg().byte()) - err |= ra_fail(output, loc, assignments.at(def.tempId()).firstloc, "Definition %d must be aligned to a full register", i); + if (def.regClass().is_subdword() && !validate_subdword_definition(program->chip_class, instr)) + err |= ra_fail(output, loc, Location(), "Definition %d not aligned correctly", i); if (!assignments[def.tempId()].firstloc.block) assignments[def.tempId()].firstloc = loc; assignments[def.tempId()].defloc = loc; @@ -602,10 +737,14 @@ bool validate_ra(Program *program, const struct radv_nir_compiler_options *optio err |= ra_fail(output, loc, assignments.at(regs[reg.reg_b + j]).defloc, "Assignment of element %d of %%%d already taken by %%%d from instruction", i, tmp.id(), regs[reg.reg_b + j]); regs[reg.reg_b + j] = tmp.id(); } - if (def.regClass().is_subdword() && !instr_can_access_subdword(program, instr)) { - for (unsigned j = tmp.bytes(); j < 4; j++) - if (regs[reg.reg_b + j]) - err |= ra_fail(output, loc, assignments.at(regs[reg.reg_b + j]).defloc, "Assignment of element %d of %%%d overwrites the full register taken by %%%d from instruction", i, tmp.id(), regs[reg.reg_b + j]); + if (def.regClass().is_subdword() && def.bytes() < 4) { + unsigned written = get_subdword_bytes_written(program, instr, i); + /* If written=4, the instruction still might write the upper half. In that case, it's the lower half that isn't preserved */ + for (unsigned j = reg.byte() & ~(written - 1); j < written; j++) { + unsigned written_reg = reg.reg() * 4u + j; + if (regs[written_reg] && regs[written_reg] != def.tempId()) + err |= ra_fail(output, loc, assignments.at(regs[written_reg]).defloc, "Assignment of element %d of %%%d overwrites the full register taken by %%%d from instruction", i, tmp.id(), regs[written_reg]); + } } } diff --git a/src/amd/compiler/meson.build b/src/amd/compiler/meson.build index dbf077b0fdc..a414e6cdb0a 100644 --- a/src/amd/compiler/meson.build +++ b/src/amd/compiler/meson.build @@ -60,6 +60,7 @@ libaco_files = files( 'aco_instruction_selection_setup.cpp', 'aco_interface.cpp', 'aco_interface.h', + 'aco_ir.cpp', 'aco_ir.h', 'aco_assembler.cpp', 'aco_insert_exec_mask.cpp',