From f57bdd48498f3284447c14c03204ac60e35c5124 Mon Sep 17 00:00:00 2001 From: Matt Turner Date: Mon, 21 Nov 2016 10:26:57 -0800 Subject: [PATCH] i965: Validate "Special Cases for Byte Operations" Do this in general_restrictions_based_on_operand_types() because the two rules that "Special Cases for Byte Operations" relax are checked there. --- src/mesa/drivers/dri/i965/brw_eu_validate.c | 70 +++++++++++++++++--- src/mesa/drivers/dri/i965/test_eu_validate.cpp | 89 ++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 9 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c b/src/mesa/drivers/dri/i965/brw_eu_validate.c index 89da831ee2b..226539445d9 100644 --- a/src/mesa/drivers/dri/i965/brw_eu_validate.c +++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c @@ -78,6 +78,34 @@ inst_is_send(const struct gen_device_info *devinfo, const brw_inst *inst) } } +static unsigned +signed_type(unsigned type) +{ + switch (type) { + case BRW_HW_REG_TYPE_UD: return BRW_HW_REG_TYPE_D; + case BRW_HW_REG_TYPE_UW: return BRW_HW_REG_TYPE_W; + case BRW_HW_REG_NON_IMM_TYPE_UB: return BRW_HW_REG_NON_IMM_TYPE_B; + case GEN8_HW_REG_TYPE_UQ: return GEN8_HW_REG_TYPE_Q; + default: return type; + } +} + +static bool +inst_is_raw_move(const struct gen_device_info *devinfo, const brw_inst *inst) +{ + unsigned dst_type = signed_type(brw_inst_dst_reg_type(devinfo, inst)); + unsigned src_type = signed_type(brw_inst_src0_reg_type(devinfo, inst)); + + if (brw_inst_src0_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE && + (brw_inst_src0_negate(devinfo, inst) || + brw_inst_src0_abs(devinfo, inst))) + return false; + + return brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV && + brw_inst_saturate(devinfo, inst) == 0 && + dst_type == src_type; +} + static bool dst_is_null(const struct gen_device_info *devinfo, const brw_inst *inst) { @@ -417,10 +445,21 @@ general_restrictions_based_on_operand_types(const struct gen_device_info *devinf if (desc->ndst == 0) return (struct string){}; - /* FINISHME: check special cases for byte operations */ - if (brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_B || - brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_UB) - return (struct string){}; + unsigned dst_stride = 1 << (brw_inst_dst_hstride(devinfo, inst) - 1); + bool dst_type_is_byte = + brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_B || + brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_UB; + + if (dst_type_is_byte) { + if (is_packed(exec_size * dst_stride, exec_size, dst_stride)) { + if (!inst_is_raw_move(devinfo, inst)) { + ERROR("Only raw MOV supports a packed-byte destination"); + return error_msg; + } else { + return (struct string){}; + } + } + } unsigned exec_type = execution_type(devinfo, inst); unsigned exec_type_size = @@ -428,17 +467,30 @@ general_restrictions_based_on_operand_types(const struct gen_device_info *devinf unsigned dst_type_size = brw_element_size(devinfo, inst, dst); if (exec_type_size > dst_type_size) { - unsigned dst_stride = 1 << (brw_inst_dst_hstride(devinfo, inst) - 1); ERROR_IF(dst_stride * dst_type_size != exec_type_size, "Destination stride must be equal to the ratio of the sizes of " "the execution data type to the destination type"); + unsigned subreg = brw_inst_dst_da1_subreg_nr(devinfo, inst); + if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1 && brw_inst_dst_address_mode(devinfo, inst) == BRW_ADDRESS_DIRECT) { - unsigned subreg = brw_inst_dst_da1_subreg_nr(devinfo, inst); - ERROR_IF(subreg % exec_type_size != 0, - "Destination subreg must be aligned to the size of the " - "execution data type"); + /* The i965 PRM says: + * + * Implementation Restriction: The relaxed alignment rule for byte + * destination (#10.5) is not supported. + */ + if ((devinfo->gen > 4 || devinfo->is_g4x) && dst_type_is_byte) { + ERROR_IF(subreg % exec_type_size != 0 && + subreg % exec_type_size != 1, + "Destination subreg must be aligned to the size of the " + "execution data type (or to the next lowest byte for byte " + "destinations)"); + } else { + ERROR_IF(subreg % exec_type_size != 0, + "Destination subreg must be aligned to the size of the " + "execution data type"); + } } } diff --git a/src/mesa/drivers/dri/i965/test_eu_validate.cpp b/src/mesa/drivers/dri/i965/test_eu_validate.cpp index 1d3eb3cb5d1..76652dc43d0 100644 --- a/src/mesa/drivers/dri/i965/test_eu_validate.cpp +++ b/src/mesa/drivers/dri/i965/test_eu_validate.cpp @@ -756,3 +756,92 @@ TEST_P(validation_test, one_src_two_dst) EXPECT_FALSE(validate(p)); } } + +TEST_P(validation_test, packed_byte_destination) +{ + static const struct { + enum brw_reg_type dst_type; + enum brw_reg_type src_type; + bool neg, abs, sat; + bool expected_result; + } move[] = { + { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UB, 0, 0, 0, true }, + { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_B , 0, 0, 0, true }, + { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_B , 0, 0, 0, true }, + { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_UB, 0, 0, 0, true }, + + { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UB, 1, 0, 0, false }, + { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_B , 1, 0, 0, false }, + { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_B , 1, 0, 0, false }, + { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_UB, 1, 0, 0, false }, + + { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UB, 0, 1, 0, false }, + { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_B , 0, 1, 0, false }, + { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_B , 0, 1, 0, false }, + { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_UB, 0, 1, 0, false }, + + { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UB, 0, 0, 1, false }, + { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_B , 0, 0, 1, false }, + { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_B , 0, 0, 1, false }, + { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_UB, 0, 0, 1, false }, + + { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UW, 0, 0, 0, false }, + { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_W , 0, 0, 0, false }, + { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UD, 0, 0, 0, false }, + { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_D , 0, 0, 0, false }, + }; + + for (unsigned i = 0; i < sizeof(move) / sizeof(move[0]); i++) { + brw_MOV(p, retype(g0, move[i].dst_type), retype(g0, move[i].src_type)); + brw_inst_set_src0_negate(&devinfo, last_inst, move[i].neg); + brw_inst_set_src0_abs(&devinfo, last_inst, move[i].abs); + brw_inst_set_saturate(&devinfo, last_inst, move[i].sat); + + EXPECT_EQ(move[i].expected_result, validate(p)); + + clear_instructions(p); + } + + brw_SEL(p, retype(g0, BRW_REGISTER_TYPE_UB), + retype(g0, BRW_REGISTER_TYPE_UB), + retype(g0, BRW_REGISTER_TYPE_UB)); + brw_inst_set_pred_control(&devinfo, last_inst, BRW_PREDICATE_NORMAL); + + EXPECT_FALSE(validate(p)); + + clear_instructions(p); + + brw_SEL(p, retype(g0, BRW_REGISTER_TYPE_B), + retype(g0, BRW_REGISTER_TYPE_B), + retype(g0, BRW_REGISTER_TYPE_B)); + brw_inst_set_pred_control(&devinfo, last_inst, BRW_PREDICATE_NORMAL); + + EXPECT_FALSE(validate(p)); +} + +TEST_P(validation_test, byte_destination_relaxed_alignment) +{ + brw_SEL(p, retype(g0, BRW_REGISTER_TYPE_B), + retype(g0, BRW_REGISTER_TYPE_W), + retype(g0, BRW_REGISTER_TYPE_W)); + brw_inst_set_pred_control(&devinfo, last_inst, BRW_PREDICATE_NORMAL); + brw_inst_set_dst_hstride(&devinfo, last_inst, BRW_HORIZONTAL_STRIDE_2); + + EXPECT_TRUE(validate(p)); + + clear_instructions(p); + + brw_SEL(p, retype(g0, BRW_REGISTER_TYPE_B), + retype(g0, BRW_REGISTER_TYPE_W), + retype(g0, BRW_REGISTER_TYPE_W)); + brw_inst_set_pred_control(&devinfo, last_inst, BRW_PREDICATE_NORMAL); + brw_inst_set_dst_hstride(&devinfo, last_inst, BRW_HORIZONTAL_STRIDE_2); + brw_inst_set_dst_da1_subreg_nr(&devinfo, last_inst, 1); + + if (devinfo.gen > 4 || devinfo.is_g4x) { + EXPECT_TRUE(validate(p)); + } else { + EXPECT_FALSE(validate(p)); + } + +} -- 2.11.0