From 9e3ff06c3804628eeac6487c9e959901358a3211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 30 Aug 2021 17:58:36 +0200 Subject: [PATCH] aco: rewrite SDWA selector This commit introduces a new struct SubdwordSel in order to ease and clean up the usage of SDWA selections. This includes removing the distinction between register-allocated and fixed SDWA selections. Instead, SDWA selections can now also access the high bits of subdword variables. Alignment and sizes are validated accordingly. Size, offset and sign_extend can be evaluated via helper methods. Reviewed-by: Rhys Perry Part-of: --- src/amd/compiler/aco_assembler.cpp | 27 ++----- .../compiler/aco_instruction_selection.cpp | 8 +- src/amd/compiler/aco_ir.cpp | 20 +---- src/amd/compiler/aco_ir.h | 77 ++++++++++-------- src/amd/compiler/aco_lower_to_hw_instr.cpp | 29 +++---- src/amd/compiler/aco_opcodes.py | 4 +- src/amd/compiler/aco_optimizer.cpp | 81 ++++++++++--------- src/amd/compiler/aco_print_ir.cpp | 46 ++++------- src/amd/compiler/aco_validate.cpp | 45 ++++++----- src/amd/compiler/tests/test_sdwa.cpp | 6 +- src/amd/compiler/tests/test_to_hw_instr.cpp | 8 +- 11 files changed, 164 insertions(+), 187 deletions(-) diff --git a/src/amd/compiler/aco_assembler.cpp b/src/amd/compiler/aco_assembler.cpp index fd4916c5dee..3ae5860d857 100644 --- a/src/amd/compiler/aco_assembler.cpp +++ b/src/amd/compiler/aco_assembler.cpp @@ -61,19 +61,6 @@ struct asm_context { int subvector_begin_pos = -1; }; -static uint32_t -get_sdwa_sel(unsigned sel, PhysReg reg) -{ - if (sel & sdwa_isra) { - unsigned size = sdwa_rasize & sel; - if (size == 1) - return reg.byte(); - else /* size == 2 */ - return sdwa_isword | (reg.byte() >> 1); - } - return sel & sdwa_asuint; -} - unsigned get_mimg_nsa_dwords(const Instruction* instr) { @@ -715,23 +702,23 @@ emit_instruction(asm_context& ctx, std::vector& out, Instruction* inst } encoding |= (sdwa.clamp ? 1 : 0) << 13; } else { - encoding |= get_sdwa_sel(sdwa.dst_sel, instr->definitions[0].physReg()) << 8; - uint32_t dst_u = sdwa.dst_sel & sdwa_sext ? 1 : 0; - if (sdwa.dst_preserve || (sdwa.dst_sel & sdwa_isra)) + encoding |= sdwa.dst_sel.to_sdwa_sel(instr->definitions[0].physReg().byte()) << 8; + uint32_t dst_u = sdwa.dst_sel.sign_extend() ? 1 : 0; + if (sdwa.dst_preserve) dst_u = 2; encoding |= dst_u << 11; encoding |= (sdwa.clamp ? 1 : 0) << 13; encoding |= sdwa.omod << 14; } - encoding |= get_sdwa_sel(sdwa.sel[0], sdwa_op.physReg()) << 16; - encoding |= sdwa.sel[0] & sdwa_sext ? 1 << 19 : 0; + encoding |= sdwa.sel[0].to_sdwa_sel(sdwa_op.physReg().byte()) << 16; + encoding |= sdwa.sel[0].sign_extend() ? 1 << 19 : 0; encoding |= sdwa.abs[0] << 21; encoding |= sdwa.neg[0] << 20; if (instr->operands.size() >= 2) { - encoding |= get_sdwa_sel(sdwa.sel[1], instr->operands[1].physReg()) << 24; - encoding |= sdwa.sel[1] & sdwa_sext ? 1 << 27 : 0; + encoding |= sdwa.sel[1].to_sdwa_sel(instr->operands[1].physReg().byte()) << 24; + encoding |= sdwa.sel[1].sign_extend() ? 1 << 27 : 0; encoding |= sdwa.abs[1] << 29; encoding |= sdwa.neg[1] << 28; } diff --git a/src/amd/compiler/aco_instruction_selection.cpp b/src/amd/compiler/aco_instruction_selection.cpp index dbe9895f5db..f8d16f0337d 100644 --- a/src/amd/compiler/aco_instruction_selection.cpp +++ b/src/amd/compiler/aco_instruction_selection.cpp @@ -644,11 +644,9 @@ convert_int(isel_context* ctx, Builder& bld, Temp src, unsigned src_bits, unsign create_instruction(aco_opcode::v_mov_b32, asSDWA(Format::VOP1), 1, 1)}; sdwa->operands[0] = Operand(src); sdwa->definitions[0] = Definition(tmp); - if (sign_extend) - sdwa->sel[0] = src_bits == 8 ? sdwa_sbyte : sdwa_sword; - else - sdwa->sel[0] = src_bits == 8 ? sdwa_ubyte : sdwa_uword; - sdwa->dst_sel = tmp.bytes() == 2 ? sdwa_uword : sdwa_udword; + sdwa->sel[0] = SubdwordSel(src_bits / 8, 0, sign_extend); + sdwa->dst_sel = tmp.bytes() == 2 ? SubdwordSel::uword : SubdwordSel::dword; + sdwa->dst_preserve = tmp.bytes() == 2; bld.insert(std::move(sdwa)); } else { assert(src_bits < 32); diff --git a/src/amd/compiler/aco_ir.cpp b/src/amd/compiler/aco_ir.cpp index a276c2e7527..5f51181da3d 100644 --- a/src/amd/compiler/aco_ir.cpp +++ b/src/amd/compiler/aco_ir.cpp @@ -276,24 +276,12 @@ convert_to_SDWA(chip_class chip, aco_ptr& instr) if (i >= 2) break; - switch (instr->operands[i].bytes()) { - case 1: sdwa.sel[i] = sdwa_ubyte; break; - case 2: sdwa.sel[i] = sdwa_uword; break; - case 4: sdwa.sel[i] = sdwa_udword; break; - } - } - switch (instr->definitions[0].bytes()) { - case 1: - sdwa.dst_sel = sdwa_ubyte; - sdwa.dst_preserve = true; - break; - case 2: - sdwa.dst_sel = sdwa_uword; - sdwa.dst_preserve = true; - break; - case 4: sdwa.dst_sel = sdwa_udword; break; + sdwa.sel[i] = SubdwordSel(instr->operands[i].bytes(), 0, false); } + sdwa.dst_sel = SubdwordSel(instr->definitions[0].bytes(), 0, false); + sdwa.dst_preserve = sdwa.dst_sel.size() < 4; + if (instr->definitions[0].getTemp().type() == RegType::sgpr && chip == GFX8) instr->definitions[0].setFixed(vcc); if (instr->definitions.size() >= 2) diff --git a/src/amd/compiler/aco_ir.h b/src/amd/compiler/aco_ir.h index c96dcce7892..624b8ff23a1 100644 --- a/src/amd/compiler/aco_ir.h +++ b/src/amd/compiler/aco_ir.h @@ -1405,40 +1405,53 @@ struct DPP_instruction : public Instruction { }; static_assert(sizeof(DPP_instruction) == sizeof(Instruction) + 8, "Unexpected padding"); -enum sdwa_sel : uint8_t { - /* masks */ - sdwa_wordnum = 0x1, - sdwa_bytenum = 0x3, - sdwa_asuint = 0x7 | 0x10, - sdwa_rasize = 0x3, +struct SubdwordSel { + enum sdwa_sel : uint8_t { + ubyte = 0x4, + uword = 0x8, + dword = 0x10, + sext = 0x20, + sbyte = ubyte | sext, + sword = uword | sext, - /* flags */ - sdwa_isword = 0x4, - sdwa_sext = 0x8, - sdwa_isra = 0x10, + ubyte0 = ubyte, + ubyte1 = ubyte | 1, + ubyte2 = ubyte | 2, + ubyte3 = ubyte | 3, + sbyte0 = sbyte, + sbyte1 = sbyte | 1, + sbyte2 = sbyte | 2, + sbyte3 = sbyte | 3, + uword0 = uword, + uword1 = uword | 2, + sword0 = sword, + sword1 = sword | 2, + }; - /* specific values */ - sdwa_ubyte0 = 0, - sdwa_ubyte1 = 1, - sdwa_ubyte2 = 2, - sdwa_ubyte3 = 3, - sdwa_uword0 = sdwa_isword | 0, - sdwa_uword1 = sdwa_isword | 1, - sdwa_udword = 6, + SubdwordSel() : sel((sdwa_sel)0) {} + constexpr SubdwordSel(sdwa_sel sel_) : sel(sel_) {} + constexpr SubdwordSel(unsigned size, unsigned offset, bool sign_extend) + : sel((sdwa_sel)((sign_extend ? sext : 0) | size << 2 | offset)) + {} + constexpr operator sdwa_sel() const { return sel; } + explicit operator bool() const { return sel != 0; } - sdwa_sbyte0 = sdwa_ubyte0 | sdwa_sext, - sdwa_sbyte1 = sdwa_ubyte1 | sdwa_sext, - sdwa_sbyte2 = sdwa_ubyte2 | sdwa_sext, - sdwa_sbyte3 = sdwa_ubyte3 | sdwa_sext, - sdwa_sword0 = sdwa_uword0 | sdwa_sext, - sdwa_sword1 = sdwa_uword1 | sdwa_sext, - sdwa_sdword = sdwa_udword | sdwa_sext, + constexpr unsigned size() const { return (sel >> 2) & 0x7; } + constexpr unsigned offset() const { return sel & 0x3; } + constexpr bool sign_extend() const { return sel & sext; } + constexpr unsigned to_sdwa_sel(unsigned reg_byte_offset) const + { + reg_byte_offset += offset(); + if (size() == 1) + return reg_byte_offset; + else if (size() == 2) + return 4 + (reg_byte_offset >> 1); + else + return 6; + } - /* register-allocated */ - sdwa_ubyte = 1 | sdwa_isra, - sdwa_uword = 2 | sdwa_isra, - sdwa_sbyte = sdwa_ubyte | sdwa_sext, - sdwa_sword = sdwa_uword | sdwa_sext, +private: + sdwa_sel sel; }; /** @@ -1452,8 +1465,8 @@ enum sdwa_sel : uint8_t { struct SDWA_instruction : public Instruction { /* these destination modifiers aren't available with VOPC except for * clamp on GFX8 */ - uint8_t sel[2]; - uint8_t dst_sel; + SubdwordSel sel[2]; + SubdwordSel dst_sel; bool neg[2]; bool abs[2]; bool dst_preserve : 1; diff --git a/src/amd/compiler/aco_lower_to_hw_instr.cpp b/src/amd/compiler/aco_lower_to_hw_instr.cpp index 1a83dc51512..e4af6fd153b 100644 --- a/src/amd/compiler/aco_lower_to_hw_instr.cpp +++ b/src/amd/compiler/aco_lower_to_hw_instr.cpp @@ -516,11 +516,9 @@ emit_reduction(lower_context* ctx, aco_opcode op, ReduceOp reduce_op, unsigned c aco_opcode::v_mov_b32, asSDWA(Format::VOP1), 1, 1)}; sdwa->operands[0] = Operand(PhysReg{tmp}, v1); sdwa->definitions[0] = Definition(PhysReg{tmp}, v1); - if (reduce_op == imin8 || reduce_op == imax8) - sdwa->sel[0] = sdwa_sbyte; - else - sdwa->sel[0] = sdwa_ubyte; - sdwa->dst_sel = sdwa_udword; + bool sext = reduce_op == imin8 || reduce_op == imax8; + sdwa->sel[0] = SubdwordSel(1, 0, sext); + sdwa->dst_sel = SubdwordSel::dword; bld.insert(std::move(sdwa)); } else { aco_opcode opcode; @@ -541,11 +539,9 @@ emit_reduction(lower_context* ctx, aco_opcode op, ReduceOp reduce_op, unsigned c aco_opcode::v_mov_b32, asSDWA(Format::VOP1), 1, 1)}; sdwa->operands[0] = Operand(PhysReg{tmp}, v1); sdwa->definitions[0] = Definition(PhysReg{tmp}, v1); - if (reduce_op == imin16 || reduce_op == imax16 || reduce_op == iadd16) - sdwa->sel[0] = sdwa_sword; - else - sdwa->sel[0] = sdwa_uword; - sdwa->dst_sel = sdwa_udword; + bool sext = reduce_op == imin16 || reduce_op == imax16 || reduce_op == iadd16; + sdwa->sel[0] = SubdwordSel(2, 0, sext); + sdwa->dst_sel = SubdwordSel::dword; bld.insert(std::move(sdwa)); } else if (ctx->program->chip_class == GFX6 || ctx->program->chip_class == GFX7) { aco_opcode opcode; @@ -2099,10 +2095,9 @@ lower_to_hw_instr(Program* program) sdwa->operands[0] = Operand(op.physReg().advance(-op.physReg().byte()), RegClass::get(op.regClass().type(), 4)); sdwa->definitions[0] = dst; - sdwa->sel[0] = sdwa_ubyte0 + op.physReg().byte() + index; - if (signext) - sdwa->sel[0] |= sdwa_sext; - sdwa->dst_sel = sdwa_uword; + sdwa->sel[0] = SubdwordSel(1, op.physReg().byte() + offset / 8, signext); + sdwa->dst_sel = SubdwordSel::uword; + sdwa->dst_preserve = true; bld.insert(std::move(sdwa)); } break; @@ -2143,8 +2138,8 @@ lower_to_hw_instr(Program* program) (Format)((uint16_t)Format::VOP1 | (uint16_t)Format::SDWA), 1, 1)}; sdwa->operands[0] = op; sdwa->definitions[0] = dst; - sdwa->sel[0] = sdwa_udword; - sdwa->dst_sel = (bits == 8 ? sdwa_ubyte0 : sdwa_uword0) + (offset / bits); + sdwa->sel[0] = SubdwordSel::dword; + sdwa->dst_sel = SubdwordSel(bits / 8, offset / 8, false); bld.insert(std::move(sdwa)); } else { bld.vop3(aco_opcode::v_bfe_u32, dst, op, Operand::zero(), Operand::c32(bits)); @@ -2157,7 +2152,7 @@ lower_to_hw_instr(Program* program) RegClass::get(op.regClass().type(), 4)); bld.vop2_sdwa(aco_opcode::v_lshlrev_b32, dst, Operand::c32(offset), sdwa_op) .instr->sdwa() - .sel[1] = sdwa_ubyte0 + op.physReg().byte(); + .sel[1] = SubdwordSel(1, op.physReg().byte(), false); } break; } diff --git a/src/amd/compiler/aco_opcodes.py b/src/amd/compiler/aco_opcodes.py index 6f860721ae5..33ba9346682 100644 --- a/src/amd/compiler/aco_opcodes.py +++ b/src/amd/compiler/aco_opcodes.py @@ -178,8 +178,8 @@ class Format(Enum): res = '' if self == Format.SDWA: for i in range(min(num_operands, 2)): - res += 'instr->sel[{0}] = op{0}.op.bytes() == 2 ? sdwa_uword : (op{0}.op.bytes() == 1 ? sdwa_ubyte : sdwa_udword);\n'.format(i) - res += 'instr->dst_sel = def0.bytes() == 2 ? sdwa_uword : (def0.bytes() == 1 ? sdwa_ubyte : sdwa_udword);\n' + res += 'instr->sel[{0}] = SubdwordSel(op{0}.op.bytes(), 0, false);'.format(i) + res += 'instr->dst_sel = SubdwordSel(def0.bytes(), 0, false);\n' res += 'if (def0.bytes() < 4) instr->dst_preserve = true;' return res diff --git a/src/amd/compiler/aco_optimizer.cpp b/src/amd/compiler/aco_optimizer.cpp index 27c993e83fc..f461932bfeb 100644 --- a/src/amd/compiler/aco_optimizer.cpp +++ b/src/amd/compiler/aco_optimizer.cpp @@ -785,36 +785,33 @@ fixed_to_exec(Operand op) return op.isFixed() && op.physReg() == exec; } -int +SubdwordSel parse_extract(Instruction* instr) { if (instr->opcode == aco_opcode::p_extract) { - bool is_byte = instr->operands[2].constantEquals(8); - unsigned index = instr->operands[1].constantValue(); - unsigned sel = (is_byte ? sdwa_ubyte0 : sdwa_uword0) + index; - if (!instr->operands[3].constantEquals(0)) - sel |= sdwa_sext; - return sel; + unsigned size = instr->operands[2].constantValue() / 8; + unsigned offset = instr->operands[1].constantValue() * size; + bool sext = instr->operands[3].constantEquals(1); + return SubdwordSel(size, offset, sext); } else if (instr->opcode == aco_opcode::p_insert && instr->operands[1].constantEquals(0)) { - return instr->operands[2].constantEquals(8) ? sdwa_ubyte0 : sdwa_uword0; + return instr->operands[2].constantEquals(8) ? SubdwordSel::ubyte : SubdwordSel::uword; } else { - return -1; + return SubdwordSel(); } } -int +SubdwordSel parse_insert(Instruction* instr) { if (instr->opcode == aco_opcode::p_extract && instr->operands[3].constantEquals(0) && instr->operands[1].constantEquals(0)) { - return instr->operands[2].constantEquals(8) ? sdwa_ubyte0 : sdwa_uword0; + return instr->operands[2].constantEquals(8) ? SubdwordSel::ubyte : SubdwordSel::uword; } else if (instr->opcode == aco_opcode::p_insert) { - bool is_byte = instr->operands[2].constantEquals(8); - unsigned index = instr->operands[1].constantValue(); - unsigned sel = (is_byte ? sdwa_ubyte0 : sdwa_uword0) + index; - return sel; + unsigned size = instr->operands[2].constantValue() / 8; + unsigned offset = instr->operands[1].constantValue() * size; + return SubdwordSel(size, offset, false); } else { - return -1; + return SubdwordSel(); } } @@ -825,20 +822,21 @@ can_apply_extract(opt_ctx& ctx, aco_ptr& instr, unsigned idx, ssa_i return false; Temp tmp = info.instr->operands[0].getTemp(); - unsigned sel = parse_extract(info.instr); + SubdwordSel sel = parse_extract(info.instr); - if (sel == sdwa_udword || sel == sdwa_sdword) { + if (!sel) { + return false; + } else if (sel.size() == 4) { return true; - } else if (instr->opcode == aco_opcode::v_cvt_f32_u32 && sel <= sdwa_ubyte3) { + } else if (instr->opcode == aco_opcode::v_cvt_f32_u32 && sel.size() == 1 && !sel.sign_extend()) { return true; } else if (can_use_SDWA(ctx.program->chip_class, instr, true) && (tmp.type() == RegType::vgpr || ctx.program->chip_class >= GFX9)) { - if (instr->isSDWA() && - (static_cast(instr.get())->sel[idx] & sdwa_asuint) != sdwa_udword) + if (instr->isSDWA() && instr->sdwa().sel[idx] != SubdwordSel::dword) return false; return true; - } else if (instr->isVOP3() && (sel & sdwa_isword) && - can_use_opsel(ctx.program->chip_class, instr->opcode, idx, (sel & sdwa_wordnum)) && + } else if (instr->isVOP3() && sel.size() == 2 && + can_use_opsel(ctx.program->chip_class, instr->opcode, idx, sel.offset()) && !(instr->vop3().opsel & (1 << idx))) { return true; } else { @@ -853,22 +851,24 @@ void apply_extract(opt_ctx& ctx, aco_ptr& instr, unsigned idx, ssa_info& info) { Temp tmp = info.instr->operands[0].getTemp(); - unsigned sel = parse_extract(info.instr); + SubdwordSel sel = parse_extract(info.instr); + assert(sel); - if (sel == sdwa_udword || sel == sdwa_sdword) { - } else if (instr->opcode == aco_opcode::v_cvt_f32_u32 && sel <= sdwa_ubyte3) { - switch (sel) { - case sdwa_ubyte0: instr->opcode = aco_opcode::v_cvt_f32_ubyte0; break; - case sdwa_ubyte1: instr->opcode = aco_opcode::v_cvt_f32_ubyte1; break; - case sdwa_ubyte2: instr->opcode = aco_opcode::v_cvt_f32_ubyte2; break; - case sdwa_ubyte3: instr->opcode = aco_opcode::v_cvt_f32_ubyte3; break; + if (sel.size() == 4) { + /* full dword selection */ + } else if (instr->opcode == aco_opcode::v_cvt_f32_u32 && sel.size() == 1 && !sel.sign_extend()) { + switch (sel.offset()) { + case 0: instr->opcode = aco_opcode::v_cvt_f32_ubyte0; break; + case 1: instr->opcode = aco_opcode::v_cvt_f32_ubyte1; break; + case 2: instr->opcode = aco_opcode::v_cvt_f32_ubyte2; break; + case 3: instr->opcode = aco_opcode::v_cvt_f32_ubyte3; break; } } else if (can_use_SDWA(ctx.program->chip_class, instr, true) && (tmp.type() == RegType::vgpr || ctx.program->chip_class >= GFX9)) { to_SDWA(ctx, instr); static_cast(instr.get())->sel[idx] = sel; } else if (instr->isVOP3()) { - if (sel & sdwa_wordnum) + if (sel.offset()) instr->vop3().opsel |= 1 << idx; } @@ -1023,7 +1023,7 @@ label_instruction(opt_ctx& ctx, aco_ptr& instr) can_use_mod = can_use_mod && instr_info.can_use_input_modifiers[(int)instr->opcode]; if (instr->isSDWA()) - can_use_mod = can_use_mod && (instr->sdwa().sel[i] & sdwa_asuint) == sdwa_udword; + can_use_mod = can_use_mod && instr->sdwa().sel[i].size() == 4; else can_use_mod = can_use_mod && (instr->isDPP() || can_use_VOP3(ctx, instr)); @@ -1673,7 +1673,7 @@ label_instruction(opt_ctx& ctx, aco_ptr& instr) case aco_opcode::p_extract: { if (instr->definitions[0].bytes() == 4) { ctx.info[instr->definitions[0].tempId()].set_extract(instr.get()); - if (instr->operands[0].regClass() == v1 && parse_insert(instr.get()) >= 0) + if (instr->operands[0].regClass() == v1 && parse_insert(instr.get())) ctx.info[instr->operands[0].tempId()].set_insert(instr.get()); } break; @@ -1682,7 +1682,7 @@ label_instruction(opt_ctx& ctx, aco_ptr& instr) if (instr->operands[0].bytes() == 4) { if (instr->operands[0].regClass() == v1) ctx.info[instr->operands[0].tempId()].set_insert(instr.get()); - if (parse_extract(instr.get()) >= 0) + if (parse_extract(instr.get())) ctx.info[instr->definitions[0].tempId()].set_extract(instr.get()); ctx.info[instr->definitions[0].tempId()].set_bitwise(instr.get()); } @@ -2868,20 +2868,21 @@ apply_insert(opt_ctx& ctx, aco_ptr& instr) /* MADs/FMAs are created later, so we don't have to update the original add */ assert(!ctx.info[instr->definitions[0].tempId()].is_mad()); - unsigned sel = parse_insert(def_info.instr); + SubdwordSel sel = parse_insert(def_info.instr); + assert(sel); - if (instr->isVOP3() && (sel & sdwa_isword) && !(sel & sdwa_sext) && - can_use_opsel(ctx.program->chip_class, instr->opcode, 3, (sel & sdwa_wordnum))) { + if (instr->isVOP3() && sel.size() == 2 && !sel.sign_extend() && + can_use_opsel(ctx.program->chip_class, instr->opcode, 3, sel.offset())) { if (instr->vop3().opsel & (1 << 3)) return false; - if (sel & sdwa_wordnum) + if (sel.offset()) instr->vop3().opsel |= 1 << 3; } else { if (!can_use_SDWA(ctx.program->chip_class, instr, true)) return false; to_SDWA(ctx, instr); - if ((static_cast(instr.get())->dst_sel & sdwa_asuint) != sdwa_udword) + if (instr->sdwa().dst_sel.size() != 4) return false; static_cast(instr.get())->dst_sel = sel; } diff --git a/src/amd/compiler/aco_print_ir.cpp b/src/amd/compiler/aco_print_ir.cpp index d48f75f3a72..15e504feb2a 100644 --- a/src/amd/compiler/aco_print_ir.cpp +++ b/src/amd/compiler/aco_print_ir.cpp @@ -629,20 +629,14 @@ print_instr_format_specific(const Instruction* instr, FILE* output) fprintf(output, " clamp"); if (instr->isVOPC()) return; - switch (sdwa.dst_sel & sdwa_asuint) { - case sdwa_udword: break; - case sdwa_ubyte0: - case sdwa_ubyte1: - case sdwa_ubyte2: - case sdwa_ubyte3: - fprintf(output, " dst_sel:%sbyte%u", sdwa.dst_sel & sdwa_sext ? "s" : "u", - sdwa.dst_sel & sdwa_bytenum); - break; - case sdwa_uword0: - case sdwa_uword1: - fprintf(output, " dst_sel:%sword%u", sdwa.dst_sel & sdwa_sext ? "s" : "u", - sdwa.dst_sel & sdwa_wordnum); - break; + if (instr->definitions[0].bytes() == 4) { + char sext = sdwa.dst_sel.sign_extend() ? 's' : 'u'; + switch (sdwa.dst_sel.size()) { + case 1: fprintf(output, " dst_sel:%cbyte%u", sext, sdwa.dst_sel.offset()); break; + case 2: fprintf(output, " dst_sel:%cword%u", sext, sdwa.dst_sel.offset() >> 1); break; + case 4: break; + default: break; + } } if (sdwa.dst_preserve) fprintf(output, " dst_preserve"); @@ -665,12 +659,12 @@ aco_print_instr(const Instruction* instr, FILE* output, unsigned flags) bool* const abs = (bool*)alloca(instr->operands.size() * sizeof(bool)); bool* const neg = (bool*)alloca(instr->operands.size() * sizeof(bool)); bool* const opsel = (bool*)alloca(instr->operands.size() * sizeof(bool)); - uint8_t* const sel = (uint8_t*)alloca(instr->operands.size() * sizeof(uint8_t)); + SubdwordSel* const sel = (SubdwordSel*)alloca(instr->operands.size() * sizeof(SubdwordSel)); for (unsigned i = 0; i < instr->operands.size(); ++i) { abs[i] = false; neg[i] = false; opsel[i] = false; - sel[i] = sdwa_udword; + sel[i] = SubdwordSel::dword; } if (instr->isVOP3()) { const VOP3_instruction& vop3 = instr->vop3(); @@ -678,7 +672,6 @@ aco_print_instr(const Instruction* instr, FILE* output, unsigned flags) abs[i] = vop3.abs[i]; neg[i] = vop3.neg[i]; opsel[i] = vop3.opsel & (1 << i); - sel[i] = sdwa_udword; } } else if (instr->isDPP()) { const DPP_instruction& dpp = instr->dpp(); @@ -686,7 +679,6 @@ aco_print_instr(const Instruction* instr, FILE* output, unsigned flags) abs[i] = dpp.abs[i]; neg[i] = dpp.neg[i]; opsel[i] = false; - sel[i] = sdwa_udword; } } else if (instr->isSDWA()) { const SDWA_instruction& sdwa = instr->sdwa(); @@ -709,21 +701,15 @@ aco_print_instr(const Instruction* instr, FILE* output, unsigned flags) fprintf(output, "|"); if (opsel[i]) fprintf(output, "hi("); - else if (sel[i] & sdwa_sext) + else if (sel[i].sign_extend()) fprintf(output, "sext("); aco_print_operand(&instr->operands[i], output, flags); - if (opsel[i] || (sel[i] & sdwa_sext)) + if (opsel[i] || (sel[i].sign_extend())) fprintf(output, ")"); - if (!(sel[i] & sdwa_isra)) { - if (sel[i] == sdwa_udword || sel[i] == sdwa_sdword) { - /* print nothing */ - } else if (sel[i] & sdwa_isword) { - unsigned index = sel[i] & sdwa_wordnum; - fprintf(output, "[%u:%u]", index * 16, index * 16 + 15); - } else { - unsigned index = sel[i] & sdwa_bytenum; - fprintf(output, "[%u:%u]", index * 8, index * 8 + 7); - } + if (instr->isSDWA() && i < 2 && sel[i].size() < 4 && instr->operands[i].bytes() == 4) { + unsigned begin = sel[i].offset() * 8; + unsigned end = begin + sel[i].size() * 8 - 1; + fprintf(output, "[%u:%u]", begin, end); } if (abs[i]) fprintf(output, "|"); diff --git a/src/amd/compiler/aco_validate.cpp b/src/amd/compiler/aco_validate.cpp index 0eb81753297..022b2fd6bab 100644 --- a/src/amd/compiler/aco_validate.cpp +++ b/src/amd/compiler/aco_validate.cpp @@ -163,16 +163,30 @@ validate_ir(Program* program) check((instr->definitions[0].isFixed() && instr->definitions[0].physReg() == vcc) || program->chip_class >= GFX9, "SDWA+VOPC definition must be fixed to vcc on GFX8", instr.get()); + } else { + const Definition& def = instr->definitions[0]; + check(def.bytes() <= 4, "SDWA definitions must not be larger than 4 bytes", + instr.get()); + check(def.bytes() >= sdwa.dst_sel.size() + sdwa.dst_sel.offset(), + "SDWA definition selection size must be at most definition size", instr.get()); + check( + sdwa.dst_sel.size() == 1 || sdwa.dst_sel.size() == 2 || sdwa.dst_sel.size() == 4, + "SDWA definition selection size must be 1, 2 or 4 bytes", instr.get()); + check(sdwa.dst_sel.offset() % sdwa.dst_sel.size() == 0, "Invalid selection offset", + instr.get()); + check(def.bytes() == 4 || sdwa.dst_preserve, + "SDWA subdword definition needs dst_preserve", instr.get()); } for (unsigned i = 0; i < std::min(2, instr->operands.size()); i++) { const Operand& op = instr->operands[i]; check(op.bytes() <= 4, "SDWA operands must not be larger than 4 bytes", instr.get()); - if (sdwa.sel[i] & sdwa_isra) - check(op.bytes() >= (sdwa.sel[i] & sdwa_rasize), - "SDWA selection size must be at most operand size", instr.get()); - else - check(op.bytes() == 4, "SDWA selection needs dword operand", instr.get()); + check(op.bytes() >= sdwa.sel[i].size() + sdwa.sel[i].offset(), + "SDWA operand selection size must be at most operand size", instr.get()); + check(sdwa.sel[i].size() == 1 || sdwa.sel[i].size() == 2 || sdwa.sel[i].size() == 4, + "SDWA operand selection size must be 1, 2 or 4 bytes", instr.get()); + check(sdwa.sel[i].offset() % sdwa.sel[i].size() == 0, "Invalid selection offset", + instr.get()); } if (instr->operands.size() >= 3) { check(instr->operands[2].isFixed() && instr->operands[2].physReg() == vcc, @@ -201,10 +215,6 @@ validate_ir(Program* program) (instr->opcode == aco_opcode::v_mac_f32 && instr->opcode == aco_opcode::v_mac_f16); check(sdwa_opcodes || feature_mac, "SDWA can't be used with this opcode", 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 */ @@ -689,10 +699,9 @@ validate_subdword_operand(chip_class chip, const aco_ptr& instr, un return byte == 0; if (instr->isPseudo() && chip >= GFX8) return true; - if (instr->isSDWA()) { - unsigned size = instr->sdwa().sel[index] & sdwa_rasize; - return byte % size == 0; - } + if (instr->isSDWA()) + return byte + instr->sdwa().sel[index].offset() + instr->sdwa().sel[index].size() <= 4 && + byte % instr->sdwa().sel[index].size() == 0; if (byte == 2 && can_use_opsel(chip, instr->opcode, index, 1)) return true; @@ -742,8 +751,9 @@ validate_subdword_definition(chip_class chip, const aco_ptr& instr) if (instr->isPseudo() && chip >= GFX8) return true; - if (instr->isSDWA() && instr->sdwa().dst_sel == (sdwa_isra | def.bytes())) - return true; + if (instr->isSDWA()) + return byte + instr->sdwa().dst_sel.offset() + instr->sdwa().dst_sel.size() <= 4 && + byte % instr->sdwa().dst_sel.size() == 0; if (byte == 2 && can_use_opsel(chip, instr->opcode, -1, 1)) return true; @@ -774,9 +784,8 @@ get_subdword_bytes_written(Program* program, const aco_ptr& instr, return chip >= GFX8 ? def.bytes() : def.size() * 4u; if (instr->isVALU()) { assert(def.bytes() <= 2); - - if (instr->isSDWA() && instr->sdwa().dst_sel == (sdwa_isra | def.bytes())) - return def.bytes(); + if (instr->isSDWA()) + return instr->sdwa().dst_sel.size(); if (instr_is_16bit(chip, instr->opcode)) return 2; diff --git a/src/amd/compiler/tests/test_sdwa.cpp b/src/amd/compiler/tests/test_sdwa.cpp index 6a16700ccf9..2bd002ed56b 100644 --- a/src/amd/compiler/tests/test_sdwa.cpp +++ b/src/amd/compiler/tests/test_sdwa.cpp @@ -39,11 +39,11 @@ BEGIN_TEST(validate.sdwa.allow) sdwa = &bld.vop2_sdwa(aco_opcode::v_mul_f32, bld.def(v1), inputs[0], inputs[1]).instr->sdwa(); sdwa->dst_preserve = true; - sdwa->dst_sel = sdwa_ubyte0; + sdwa->dst_sel = SubdwordSel::ubyte0; sdwa = &bld.vop2_sdwa(aco_opcode::v_mul_f32, bld.def(v1), inputs[0], inputs[1]).instr->sdwa(); - sdwa->sel[0] = sdwa_sbyte2; - sdwa->sel[1] = sdwa_uword1; + sdwa->sel[0] = SubdwordSel::sbyte2; + sdwa->sel[1] = SubdwordSel::uword1; finish_validator_test(); } diff --git a/src/amd/compiler/tests/test_to_hw_instr.cpp b/src/amd/compiler/tests/test_to_hw_instr.cpp index 4c9b55a13a2..83975a2a643 100644 --- a/src/amd/compiler/tests/test_to_hw_instr.cpp +++ b/src/amd/compiler/tests/test_to_hw_instr.cpp @@ -554,15 +554,15 @@ BEGIN_TEST(to_hw_instr.extract) //>> p_unit_test 4 bld.pseudo(aco_opcode::p_unit_test, Operand::c32(4u)); //~gfx7.*! v2b: %_:v[0][0:16] = @v_bfe %_:v[1][0:16], 0, 8 - //~gfx[^7].*! v2b: %_:v[0][0:16] = v_mov_b32 @sel(0:7) + //~gfx[^7].*! v2b: %_:v[0][0:16] = v_mov_b32 @sel(0:7) dst_preserve EXT(0, 0) - //~gfx[^7].*! v2b: %_:v[0][0:16] = v_mov_b32 @sel(16:23) + //~gfx[^7].*! v2b: %_:v[0][0:16] = v_mov_b32 @sel(16:23) dst_preserve if (i != GFX7) EXT(0, 2) //~gfx7.*! v2b: %_:v[0][0:16] = @v_bfe %_:v[1][0:16], 8, 8 - //~gfx[^7].*! v2b: %_:v[0][0:16] = v_mov_b32 @sel(8:15) + //~gfx[^7].*! v2b: %_:v[0][0:16] = v_mov_b32 @sel(8:15) dst_preserve EXT(1, 0) - //~gfx[^7].*! v2b: %_:v[0][0:16] = v_mov_b32 @sel(24:31) + //~gfx[^7].*! v2b: %_:v[0][0:16] = v_mov_b32 @sel(24:31) dst_preserve if (i != GFX7) EXT(1, 2)