From 0f3a3500877eafb34741c405672d2c7829f5e389 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Mon, 29 Jul 2024 12:59:28 -0700 Subject: [PATCH] brw/nir: Don't generate scalar byte to float conversions on DG2+ in optimize_extract_to_float The lowering code does not generate efficient code. It is better to just not emit the bad thing in the first place. The shaders that I examined had blocks of NIR like: con 32 %527 = extract_u8 %456.o, %5 (0x0) con 32 %528 = extract_u8 %456.o, %35 (0x1) con 32 %529 = extract_u8 %456.o, %14 (0x2) con 32 %530 = extract_u8 %456.o, %11 (0x3) con 32 %531 = u2f32 %527 con 32 %532 = u2f32 %528 con 32 %533 = u2f32 %529 con 32 %534 = u2f32 %530 In some cases the u2f results are multiplied with 1/255. There may be a slightly more efficient way to do this by doing something like mov(8) g40<1>UW g12.1<32,8,4>UB mov(8) g41<1>UW g12.2<32,8,4>UB mov(8) g42<1>UW g12.3<32,8,4>UB mov(8) g60<1>F g12<32,8,4>UB mov(8) g61<1>F g40<1,1,0>UW mov(8) g62<1>F g41<1,1,0>UW mov(8) g63<1>F g42<1,1,0>UW In SIMD16 and SIMD32 that would save temporary register space. It could save a register in SIMD8 by using g40.8 instead of g42. Making that happen might be tricky. Maybe we should just add a special NIR opcode that converts a packed uint32 to a vec4? v2: Add a bunch of documentation explaining what's going on. Suggested by Ken. shader-db: Lunar Lake, Meteor Lake, and DG2 had similar results. (Lunar Lake shown) total instructions in shared programs: 18228689 -> 18228720 (<.01%) instructions in affected programs: 43091 -> 43122 (0.07%) helped: 0 / HURT: 30 total cycles in shared programs: 932542994 -> 932544290 (<.01%) cycles in affected programs: 8150758 -> 8152054 (0.02%) helped: 15 / HURT: 17 fossil-db: Lunar Lake, Meteor Lake, and DG2 had similar results. (Lunar Lake shown) Totals: Instrs: 142890605 -> 142890392 (-0.00%); split: -0.00%, +0.00% Cycle count: 21655049536 -> 21654693720 (-0.00%); split: -0.00%, +0.00% Totals from 181 (0.03% of 553251) affected shaders: Instrs: 188022 -> 187809 (-0.11%); split: -0.12%, +0.01% Cycle count: 85291658 -> 84935842 (-0.42%); split: -0.47%, +0.05% Tiger Lake, Ice Lake, and Skylake had similar results. (Tiger Lake shown) Totals: Instrs: 154438050 -> 154436980 (-0.00%) Cycle count: 15334650326 -> 15334644375 (-0.00%); split: -0.00%, +0.00% Spill count: 56754 -> 56706 (-0.08%) Fill count: 95919 -> 95808 (-0.12%) Scratch Memory Size: 2306048 -> 2304000 (-0.09%) Max live registers: 32469924 -> 32469899 (-0.00%) Totals from 112 (0.02% of 642922) affected shaders: Instrs: 156186 -> 155116 (-0.69%) Cycle count: 11111478 -> 11105527 (-0.05%); split: -0.62%, +0.56% Spill count: 1766 -> 1718 (-2.72%) Fill count: 2815 -> 2704 (-3.94%) Scratch Memory Size: 78848 -> 76800 (-2.60%) Max live registers: 11526 -> 11501 (-0.22%) Reviewed-by: Kenneth Graunke Part-of: --- src/intel/compiler/brw_fs_nir.cpp | 38 +++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/intel/compiler/brw_fs_nir.cpp b/src/intel/compiler/brw_fs_nir.cpp index 545e1bf18b1..fc34800df72 100644 --- a/src/intel/compiler/brw_fs_nir.cpp +++ b/src/intel/compiler/brw_fs_nir.cpp @@ -567,8 +567,46 @@ optimize_extract_to_float(nir_to_brw_state &ntb, const fs_builder &bld, op0.type = brw_type_for_nir_type(devinfo, (nir_alu_type)(nir_op_infos[src0->op].input_types[0] | nir_src_bit_size(src0->src[0].src))); + + /* It is not documented in the Bspec, but DG2 and newer platforms cannot do + * direct byte-to-float conversions from scalars. MR !30140 has more + * details. If the optimization is applied in cases that would require + * lower_regioning to do some lowering, the code generated will be much, + * much worse. + */ + if (devinfo->verx10 >= 125 && bytes == 1) { + /* If the source truly scalar, for example from the UNIFORM file, skip + * the optimize_extract_to_float optimization. + * + * Note: is_scalar values won't have zero stride until after the call to + * offset() below that applies the swizzle. + */ + if (is_uniform(op0)) + return false; + + /* If the dispatch width matches the scalar allocation width, then + * is_scalar can be demoted to non-is_scalar. This prevents offset() and + * component() (both called below) from setting the stride to zero, and + * that avoids the awful code generated by lower_regioning. + */ + if (op0.is_scalar) { + const unsigned allocation_width = 8 * reg_unit(ntb.devinfo); + if (ntb.bld.dispatch_width() != allocation_width) + return false; + + assert(bld.dispatch_width() == allocation_width); + op0.is_scalar = false; + } + } + op0 = offset(op0, bld, src0->src[0].swizzle[0]); + /* If the dispatch width matches the scalar allocation width, offset() will + * not modify the stride, but having source stride <0;1,0> is advantageous. + */ + if (op0.is_scalar) + op0 = component(op0, 0); + /* Bspec "Register Region Restrictions" for Xe says: * * "In case of all float point data types used in destination