Re: [Mesa-dev] [PATCH v4 35/40] intel/compiler: validate conversions between 64-bit and 8-bit types
On Tue, 2019-02-26 at 15:50 -0800, Francisco Jerez wrote: > Iago Toral Quiroga writes: > > > --- > > src/intel/compiler/brw_eu_validate.c| 10 +- > > src/intel/compiler/test_eu_validate.cpp | 46 > > + > > 2 files changed, 55 insertions(+), 1 deletion(-) > > > > diff --git a/src/intel/compiler/brw_eu_validate.c > > b/src/intel/compiler/brw_eu_validate.c > > index 203641fecb9..b1fdd1ce941 100644 > > --- a/src/intel/compiler/brw_eu_validate.c > > +++ b/src/intel/compiler/brw_eu_validate.c > > @@ -533,10 +533,18 @@ > > general_restrictions_based_on_operand_types(const struct > > gen_device_info *devinf > > > > /* From the BDW+ PRM: > > * > > -*"There is no direct conversion from HF to DF or DF to HF. > > +*"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. > > +* There is no direct conversion from HF to DF or DF to HF. > > * There is no direct conversion from HF to Q/UQ or Q/UQ to > > HF." > > */ > > enum brw_reg_type src0_type = brw_inst_src0_type(devinfo, > > inst); > > + > > + ERROR_IF(brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV && > > +((dst_type_size == 1 && type_sz(src0_type) == 8) || > > + (dst_type_size == 8 && type_sz(src0_type) == 1)), > > +"There are no direct conversion between 64-bit types > > and B/UB"); > > + > > Same comment here as for the last patch -- Why is this only handling > the > MOV instruction? As in the last patch, these restrictions are included in the programming notes for the MOV instructions, so they are specific to it. > > > ERROR_IF(brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV && > > ((dst_type == BRW_REGISTER_TYPE_HF && > > type_sz(src0_type) == 8) || > > (dst_type_size == 8 && src0_type == > > BRW_REGISTER_TYPE_HF)), > > diff --git a/src/intel/compiler/test_eu_validate.cpp > > b/src/intel/compiler/test_eu_validate.cpp > > index 1557b6d2452..06beb53eb5d 100644 > > --- a/src/intel/compiler/test_eu_validate.cpp > > +++ b/src/intel/compiler/test_eu_validate.cpp > > @@ -848,6 +848,52 @@ 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(B, Q, 2, false), > > + INST(B, UQ, 2, false), > > + INST(B, DF, 2, false), > > + > > + INST(B, Q, 4, false), > > + INST(B, UQ, 4, false), > > + INST(B, 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(, 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.17.1 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 35/40] intel/compiler: validate conversions between 64-bit and 8-bit types
Iago Toral Quiroga writes: > --- > src/intel/compiler/brw_eu_validate.c| 10 +- > src/intel/compiler/test_eu_validate.cpp | 46 + > 2 files changed, 55 insertions(+), 1 deletion(-) > > diff --git a/src/intel/compiler/brw_eu_validate.c > b/src/intel/compiler/brw_eu_validate.c > index 203641fecb9..b1fdd1ce941 100644 > --- a/src/intel/compiler/brw_eu_validate.c > +++ b/src/intel/compiler/brw_eu_validate.c > @@ -533,10 +533,18 @@ general_restrictions_based_on_operand_types(const > struct gen_device_info *devinf > > /* From the BDW+ PRM: > * > -*"There is no direct conversion from HF to DF or DF to HF. > +*"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. > +* There is no direct conversion from HF to DF or DF to HF. > * There is no direct conversion from HF to Q/UQ or Q/UQ to HF." > */ > enum brw_reg_type src0_type = brw_inst_src0_type(devinfo, inst); > + > + ERROR_IF(brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV && > +((dst_type_size == 1 && type_sz(src0_type) == 8) || > + (dst_type_size == 8 && type_sz(src0_type) == 1)), > +"There are no direct conversion between 64-bit types and B/UB"); > + Same comment here as for the last patch -- Why is this only handling the MOV instruction? > ERROR_IF(brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV && > ((dst_type == BRW_REGISTER_TYPE_HF && type_sz(src0_type) == 8) || > (dst_type_size == 8 && src0_type == BRW_REGISTER_TYPE_HF)), > diff --git a/src/intel/compiler/test_eu_validate.cpp > b/src/intel/compiler/test_eu_validate.cpp > index 1557b6d2452..06beb53eb5d 100644 > --- a/src/intel/compiler/test_eu_validate.cpp > +++ b/src/intel/compiler/test_eu_validate.cpp > @@ -848,6 +848,52 @@ 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(B, Q, 2, false), > + INST(B, UQ, 2, false), > + INST(B, DF, 2, false), > + > + INST(B, Q, 4, false), > + INST(B, UQ, 4, false), > + INST(B, 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(, 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.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 35/40] intel/compiler: validate conversions between 64-bit and 8-bit types
On Sat, 2019-02-16 at 09:42 -0600, Jason Ekstrand wrote: > On Tue, Feb 12, 2019 at 5:56 AM Iago Toral Quiroga > wrote: > > --- > > > > src/intel/compiler/brw_eu_validate.c| 10 +- > > > > src/intel/compiler/test_eu_validate.cpp | 46 > > + > > > > 2 files changed, 55 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/src/intel/compiler/brw_eu_validate.c > > b/src/intel/compiler/brw_eu_validate.c > > > > index 203641fecb9..b1fdd1ce941 100644 > > > > --- a/src/intel/compiler/brw_eu_validate.c > > > > +++ b/src/intel/compiler/brw_eu_validate.c > > > > @@ -533,10 +533,18 @@ > > general_restrictions_based_on_operand_types(const struct > > gen_device_info *devinf > > > > > > > > /* From the BDW+ PRM: > > > > * > > > > -*"There is no direct conversion from HF to DF or DF to HF. > > > > +*"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. > > > > +* There is no direct conversion from HF to DF or DF to HF. > > > > * There is no direct conversion from HF to Q/UQ or Q/UQ to > > HF." > > > > */ > > > > enum brw_reg_type src0_type = brw_inst_src0_type(devinfo, > > inst); > > > > + > > > > + ERROR_IF(brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV && > > > > +((dst_type_size == 1 && type_sz(src0_type) == 8) || > > > > + (dst_type_size == 8 && type_sz(src0_type) == 1)), > > > > +"There are no direct conversion between 64-bit types > > and B/UB"); > > > > + > > > > ERROR_IF(brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV && > > > > ((dst_type == BRW_REGISTER_TYPE_HF && > > type_sz(src0_type) == 8) || > > > > (dst_type_size == 8 && src0_type == > > BRW_REGISTER_TYPE_HF)), > > > > diff --git a/src/intel/compiler/test_eu_validate.cpp > > b/src/intel/compiler/test_eu_validate.cpp > > > > index 1557b6d2452..06beb53eb5d 100644 > > > > --- a/src/intel/compiler/test_eu_validate.cpp > > > > +++ b/src/intel/compiler/test_eu_validate.cpp > > > > @@ -848,6 +848,52 @@ 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(B, Q, 2, false), > > > > + INST(B, UQ, 2, false), > > > > + INST(B, DF, 2, false), > > > > + > > > > + INST(B, Q, 4, false), > > > > + INST(B, UQ, 4, false), > > > > + INST(B, DF, 4, false), > > Probably want some tests with a B or UB source too. :-) With those > added, Sure, I added UB variants for all tests locally. > Reviewed-by: Jason Ekstrand > > > + > > > > +#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(, 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 { > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 35/40] intel/compiler: validate conversions between 64-bit and 8-bit types
On Tue, Feb 12, 2019 at 5:56 AM Iago Toral Quiroga wrote: > --- > src/intel/compiler/brw_eu_validate.c| 10 +- > src/intel/compiler/test_eu_validate.cpp | 46 + > 2 files changed, 55 insertions(+), 1 deletion(-) > > diff --git a/src/intel/compiler/brw_eu_validate.c > b/src/intel/compiler/brw_eu_validate.c > index 203641fecb9..b1fdd1ce941 100644 > --- a/src/intel/compiler/brw_eu_validate.c > +++ b/src/intel/compiler/brw_eu_validate.c > @@ -533,10 +533,18 @@ general_restrictions_based_on_operand_types(const > struct gen_device_info *devinf > > /* From the BDW+ PRM: > * > -*"There is no direct conversion from HF to DF or DF to HF. > +*"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. > +* There is no direct conversion from HF to DF or DF to HF. > * There is no direct conversion from HF to Q/UQ or Q/UQ to HF." > */ > enum brw_reg_type src0_type = brw_inst_src0_type(devinfo, inst); > + > + ERROR_IF(brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV && > +((dst_type_size == 1 && type_sz(src0_type) == 8) || > + (dst_type_size == 8 && type_sz(src0_type) == 1)), > +"There are no direct conversion between 64-bit types and > B/UB"); > + > ERROR_IF(brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV && > ((dst_type == BRW_REGISTER_TYPE_HF && type_sz(src0_type) == > 8) || > (dst_type_size == 8 && src0_type == BRW_REGISTER_TYPE_HF)), > diff --git a/src/intel/compiler/test_eu_validate.cpp > b/src/intel/compiler/test_eu_validate.cpp > index 1557b6d2452..06beb53eb5d 100644 > --- a/src/intel/compiler/test_eu_validate.cpp > +++ b/src/intel/compiler/test_eu_validate.cpp > @@ -848,6 +848,52 @@ 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(B, Q, 2, false), > + INST(B, UQ, 2, false), > + INST(B, DF, 2, false), > + > + INST(B, Q, 4, false), > + INST(B, UQ, 4, false), > + INST(B, DF, 4, false), > Probably want some tests with a B or UB source too. :-) With those added, Reviewed-by: Jason Ekstrand > + > +#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(, 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.17.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev