From 1d9e2b761a790c2873a71defb95118c0cfd2ebbc Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Wed, 27 Mar 2024 15:56:13 -0700 Subject: [PATCH] intel/brw: Update comments for indirect MOV splitting brw_broadcast and generate_mov_indirect both had similar comments, both with typos ("insead"). One still referred to IVB bugs, while the other dropped that during the compiler split. The one that dropped the comment mentioned "both of these" issues, while citing only one issue; there was in fact a third issue (no-Q/UQ) that wasn't mentioned in either comment. One also had some bad grammar in the comments. Reviewed-by: Ian Romanick Part-of: --- src/intel/compiler/brw_eu_emit.c | 14 ++++++++------ src/intel/compiler/brw_fs_generator.cpp | 20 +++++++++----------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c index 90de163478e..0499be04260 100644 --- a/src/intel/compiler/brw_eu_emit.c +++ b/src/intel/compiler/brw_eu_emit.c @@ -2052,15 +2052,17 @@ brw_broadcast(struct brw_codegen *p, !devinfo->has_64bit_int)) { /* From the Cherryview PRM Vol 7. "Register Region Restrictions": * - * "When source or destination datatype is 64b or operation is + * "When source or destination datatype is 64b or operation is * integer DWord multiply, indirect addressing must not be * used." * - * To work around both of this issue, we do two integer MOVs - * insead of one 64-bit MOV. Because no double value should ever - * cross a register boundary, it's safe to use the immediate - * offset in the indirect here to handle adding 4 bytes to the - * offset and avoid the extra ADD to the register file. + * We may also not support Q/UQ types. + * + * To work around both of these, we do two integer MOVs instead + * of one 64-bit MOV. Because no double value should ever cross + * a register boundary, it's safe to use the immediate offset in + * the indirect here to handle adding 4 bytes to the offset and + * avoid the extra ADD to the register file. */ brw_MOV(p, subscript(dst, BRW_REGISTER_TYPE_D, 0), retype(brw_vec1_indirect(addr.subnr, offset), diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp index c69a487dccf..3f9554182b5 100644 --- a/src/intel/compiler/brw_fs_generator.cpp +++ b/src/intel/compiler/brw_fs_generator.cpp @@ -380,20 +380,18 @@ fs_generator::generate_mov_indirect(fs_inst *inst, if (type_sz(reg.type) > 4 && (intel_device_info_is_9lp(devinfo) || !devinfo->has_64bit_int)) { - /* IVB has an issue (which we found empirically) where it reads two - * address register components per channel for indirectly addressed - * 64-bit sources. + /* From the Cherryview PRM Vol 7. "Register Region Restrictions": * - * From the Cherryview PRM Vol 7. "Register Region Restrictions": - * - * "When source or destination datatype is 64b or operation is + * "When source or destination datatype is 64b or operation is * integer DWord multiply, indirect addressing must not be used." * - * To work around both of these, we do two integer MOVs insead of one - * 64-bit MOV. Because no double value should ever cross a register - * boundary, it's safe to use the immediate offset in the indirect - * here to handle adding 4 bytes to the offset and avoid the extra - * ADD to the register file. + * We may also not support Q/UQ types. + * + * To work around both of these, we do two integer MOVs instead + * of one 64-bit MOV. Because no double value should ever cross + * a register boundary, it's safe to use the immediate offset in + * the indirect here to handle adding 4 bytes to the offset and + * avoid the extra ADD to the register file. */ brw_MOV(p, subscript(dst, BRW_REGISTER_TYPE_D, 0), retype(brw_VxH_indirect(0, 0), BRW_REGISTER_TYPE_D));