From 58d6417e591db3f440c4a1c06c9cfdfae2a06dfb Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Fri, 8 Feb 2019 09:20:56 +0100 Subject: [PATCH] intel/compiler: validate conversions between 64-bit and 8-bit types v2: - Add some tests with UB type too (Jason) v3: - consider implicit conversions from 2src instructions too (Curro). v4: - Do not check src1 type in single-source instructions (Curro). Reviewed-by: Jason Ekstrand (v2) --- src/intel/compiler/brw_eu_validate.c | 50 ++++++++++++++++++++++ src/intel/compiler/test_eu_validate.cpp | 55 +++++++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/src/intel/compiler/brw_eu_validate.c b/src/intel/compiler/brw_eu_validate.c index 54bffb3af03..cfaf126e2f5 100644 --- a/src/intel/compiler/brw_eu_validate.c +++ b/src/intel/compiler/brw_eu_validate.c @@ -529,6 +529,31 @@ is_mixed_float(const struct gen_device_info *devinfo, const brw_inst *inst) types_are_mixed_float(src1_type, dst_type); } +/** + * Returns whether an instruction is an explicit or implicit conversion + * to/from byte. + */ +static bool +is_byte_conversion(const struct gen_device_info *devinfo, + const brw_inst *inst) +{ + enum brw_reg_type dst_type = brw_inst_dst_type(devinfo, inst); + + unsigned num_sources = num_sources_from_inst(devinfo, inst); + enum brw_reg_type src0_type = brw_inst_src0_type(devinfo, inst); + + if (dst_type != src0_type && + (type_sz(dst_type) == 1 || type_sz(src0_type) == 1)) { + return true; + } else if (num_sources > 1) { + enum brw_reg_type src1_type = brw_inst_src1_type(devinfo, inst); + return dst_type != src1_type && + (type_sz(dst_type) == 1 || type_sz(src1_type) == 1); + } + + return false; +} + /** * Checks restrictions listed in "General Restrictions Based on Operand Types" * in the "Register Region Restrictions" section. @@ -599,6 +624,31 @@ general_restrictions_based_on_operand_types(const struct gen_device_info *devinf exec_type_size == 8 && dst_type_size == 4) dst_type_size = 8; + if (is_byte_conversion(devinfo, inst)) { + /* From the BDW+ PRM, Volume 2a, Command Reference, Instructions - MOV: + * + * "There is no direct conversion from B/UB to DF or DF to B/UB. + * There is no direct conversion from B/UB to Q/UQ or Q/UQ to B/UB." + * + * Even if these restrictions are listed for the MOV instruction, we + * validate this more generally, since there is the possibility + * of implicit conversions from other instructions. + */ + enum brw_reg_type src0_type = brw_inst_src0_type(devinfo, inst); + enum brw_reg_type src1_type = num_sources > 1 ? + brw_inst_src1_type(devinfo, inst) : 0; + + ERROR_IF(type_sz(dst_type) == 1 && + (type_sz(src0_type) == 8 || + (num_sources > 1 && type_sz(src1_type) == 8)), + "There are no direct conversions between 64-bit types and B/UB"); + + ERROR_IF(type_sz(dst_type) == 8 && + (type_sz(src0_type) == 1 || + (num_sources > 1 && type_sz(src1_type) == 1)), + "There are no direct conversions between 64-bit types and B/UB"); + } + if (is_half_float_conversion(devinfo, inst)) { /** * A helper to validate used in the validation of the following restriction diff --git a/src/intel/compiler/test_eu_validate.cpp b/src/intel/compiler/test_eu_validate.cpp index 3fdbecb003b..2e06da2f5b4 100644 --- a/src/intel/compiler/test_eu_validate.cpp +++ b/src/intel/compiler/test_eu_validate.cpp @@ -848,6 +848,61 @@ TEST_P(validation_test, byte_destination_relaxed_alignment) } } +TEST_P(validation_test, byte_64bit_conversion) +{ + static const struct { + enum brw_reg_type dst_type; + enum brw_reg_type src_type; + unsigned dst_stride; + bool expected_result; + } inst[] = { +#define INST(dst_type, src_type, dst_stride, expected_result) \ + { \ + BRW_REGISTER_TYPE_##dst_type, \ + BRW_REGISTER_TYPE_##src_type, \ + BRW_HORIZONTAL_STRIDE_##dst_stride, \ + expected_result, \ + } + + INST(B, Q, 1, false), + INST(B, UQ, 1, false), + INST(B, DF, 1, false), + INST(UB, Q, 1, false), + INST(UB, UQ, 1, false), + INST(UB, DF, 1, false), + + INST(B, Q, 2, false), + INST(B, UQ, 2, false), + INST(B , DF, 2, false), + INST(UB, Q, 2, false), + INST(UB, UQ, 2, false), + INST(UB, DF, 2, false), + + INST(B, Q, 4, false), + INST(B, UQ, 4, false), + INST(B, DF, 4, false), + INST(UB, Q, 4, false), + INST(UB, UQ, 4, false), + INST(UB, DF, 4, false), + +#undef INST + }; + + if (devinfo.gen < 8) + return; + + for (unsigned i = 0; i < sizeof(inst) / sizeof(inst[0]); i++) { + if (!devinfo.has_64bit_types && type_sz(inst[i].src_type) == 8) + continue; + + brw_MOV(p, retype(g0, inst[i].dst_type), retype(g0, inst[i].src_type)); + brw_inst_set_dst_hstride(&devinfo, last_inst, inst[i].dst_stride); + EXPECT_EQ(inst[i].expected_result, validate(p)); + + clear_instructions(p); + } +} + TEST_P(validation_test, half_float_conversion) { static const struct { -- 2.30.2