From 8361cd4c4cf7c5e63e7d1be3a7b57898e9de62bc Mon Sep 17 00:00:00 2001 From: Sviatoslav Peleshko Date: Thu, 28 Sep 2023 20:26:06 +0300 Subject: [PATCH] intel/eu/validate: Validate "packed word exception" stricter Fixes: 75b7f5a2 ("i965: Validate "Region Alignment Rules"") Signed-off-by: Sviatoslav Peleshko Reviewed-by: Emma Anholt Part-of: --- src/intel/compiler/brw_eu_validate.c | 14 ++++++++++++-- src/intel/compiler/test_eu_validate.cpp | 30 +++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/intel/compiler/brw_eu_validate.c b/src/intel/compiler/brw_eu_validate.c index f9383af..d105f6b 100644 --- a/src/intel/compiler/brw_eu_validate.c +++ b/src/intel/compiler/brw_eu_validate.c @@ -1678,6 +1678,16 @@ region_alignment_rules(const struct brw_isa_info *isa, * * Additionally the simulator source code indicates that the real condition * is that the size of the destination type is 4 bytes. + * + * HSW PRMs also add a note to the second exception: + * "When lower 8 channels are disabled, the sub register of source1 + * operand is not incremented. If the lower 8 channels are expected + * to be disabled, say by predication, the instruction must be split + * into pair of simd8 operations." + * + * We can't reliably know if the channels won't be disabled due to, + * for example, IMASK. So, play it safe and disallow packed-word exception + * for src1. */ if (devinfo->ver <= 7 && dst_regs == 2) { enum brw_reg_type dst_type = inst_dst_type(isa, inst); @@ -1692,7 +1702,7 @@ region_alignment_rules(const struct brw_isa_info *isa, width = WIDTH(brw_inst_src ## n ## _width(devinfo, inst)); \ hstride = STRIDE(brw_inst_src ## n ## _hstride(devinfo, inst)); \ bool src ## n ## _is_packed_word = \ - is_packed(vstride, width, hstride) && \ + n != 1 && is_packed(vstride, width, hstride) && \ (brw_inst_src ## n ## _type(devinfo, inst) == BRW_REGISTER_TYPE_W || \ brw_inst_src ## n ## _type(devinfo, inst) == BRW_REGISTER_TYPE_UW); \ \ @@ -1701,7 +1711,7 @@ region_alignment_rules(const struct brw_isa_info *isa, !(dst_is_packed_dword && src ## n ## _is_packed_word), \ "When the destination spans two registers, the source must " \ "span two registers\n" ERROR_INDENT "(exceptions for scalar " \ - "source and packed-word to packed-dword expansion)") + "sources, and packed-word to packed-dword expansion for src0)") if (i == 0) { DO_SRC(0); diff --git a/src/intel/compiler/test_eu_validate.cpp b/src/intel/compiler/test_eu_validate.cpp index 1a78a29..ceb5550 100644 --- a/src/intel/compiler/test_eu_validate.cpp +++ b/src/intel/compiler/test_eu_validate.cpp @@ -1130,7 +1130,7 @@ TEST_P(validation_test, one_src_two_dst) brw_inst_set_exec_size(&devinfo, last_inst, BRW_EXECUTE_16); brw_inst_set_dst_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_D); brw_inst_set_src0_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_W); - brw_inst_set_src1_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_W); + brw_inst_set_src1_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_D); EXPECT_TRUE(validate(p)); @@ -1138,6 +1138,34 @@ TEST_P(validation_test, one_src_two_dst) brw_ADD(p, g0, g0, g0); brw_inst_set_exec_size(&devinfo, last_inst, BRW_EXECUTE_16); + brw_inst_set_dst_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_D); + brw_inst_set_src0_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_D); + brw_inst_set_src1_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_W); + + if (devinfo.ver >= 8) { + EXPECT_TRUE(validate(p)); + } else { + EXPECT_FALSE(validate(p)); + } + + clear_instructions(p); + + brw_ADD(p, g0, g0, g0); + brw_inst_set_exec_size(&devinfo, last_inst, BRW_EXECUTE_16); + brw_inst_set_dst_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_D); + brw_inst_set_src0_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_W); + brw_inst_set_src1_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_W); + + if (devinfo.ver >= 8) { + EXPECT_TRUE(validate(p)); + } else { + EXPECT_FALSE(validate(p)); + } + + clear_instructions(p); + + brw_ADD(p, g0, g0, g0); + brw_inst_set_exec_size(&devinfo, last_inst, BRW_EXECUTE_16); brw_inst_set_dst_hstride(&devinfo, last_inst, BRW_HORIZONTAL_STRIDE_2); brw_inst_set_dst_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_W); brw_inst_set_src0_file_type(&devinfo, last_inst, BRW_GENERAL_REGISTER_FILE, BRW_REGISTER_TYPE_W); -- 2.7.4