Matt Turner <[email protected]> writes: > On Mon, Jan 8, 2018 at 5:01 PM, Scott D Phillips > <[email protected]> wrote: >> Matt Turner <[email protected]> writes: >> >>> Some cases weren't handled, such as stride 4 which is needed for 64-bit >>> operations. Presumably fixes the assertion failure mentioned in commit >>> 2d0457203871 (Revert "i965/fs: Use align1 mode on ternary instructions >>> on Gen10+") but who can really say since the commit neglected to list >>> any of them! >>> --- >>> src/intel/compiler/brw_eu_emit.c | 69 >>> ++++++++++++++++++++++++---------------- >>> 1 file changed, 41 insertions(+), 28 deletions(-) >>> >>> diff --git a/src/intel/compiler/brw_eu_emit.c >>> b/src/intel/compiler/brw_eu_emit.c >>> index 85bb6a4cdd..c25d8d6eda 100644 >>> --- a/src/intel/compiler/brw_eu_emit.c >>> +++ b/src/intel/compiler/brw_eu_emit.c >>> @@ -673,6 +673,42 @@ get_3src_subreg_nr(struct brw_reg reg) >>> return reg.subnr / 4; >>> } >>> >>> +static enum gen10_align1_3src_vertical_stride >>> +to_3src_align1_vstride(enum brw_vertical_stride vstride) >>> +{ >>> + switch (vstride) { >>> + case BRW_VERTICAL_STRIDE_0: >>> + return BRW_ALIGN1_3SRC_VERTICAL_STRIDE_0; >>> + case BRW_VERTICAL_STRIDE_2: >>> + return BRW_ALIGN1_3SRC_VERTICAL_STRIDE_2; >>> + case BRW_VERTICAL_STRIDE_4: >>> + return BRW_ALIGN1_3SRC_VERTICAL_STRIDE_4; >>> + case BRW_VERTICAL_STRIDE_8: >>> + case BRW_VERTICAL_STRIDE_16: >>> + return BRW_ALIGN1_3SRC_VERTICAL_STRIDE_8; >> >> What is the reasoning for vstride 16 to map to 8 here? Could that cause >> problems? > > Good question. In an exec_size 16 instruction, a 16,16,1 region > actually reads the same channels in the same order as an 8,8,1 region > (another confusing thing about regioning is that there are effectively > duplicates...). That's the most common region, and I seem to recall > that in some cases the IR contains instructions with a 16,16,1 region. > Other than in that duplicate case, I don't think we ever use vstride > 16. As a result, they left it out of the hardware for align1 ternary > instructions. > > If I can get some hardware to test with, it's probably a good idea for > me to to double check which cases cause us to need to handle vstride > 16 here. Maybe an assertion that it is the "duplicate" 16,16,1 region > should be added.
Got it, with the assert or the double check on your todo list, patch is Reviewed-by: Scott D Phillips <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
