From b54b67e067da6ed22a7b8112cb6f8bed0e188272 Mon Sep 17 00:00:00 2001 From: Francisco Jerez Date: Fri, 3 Jan 2020 17:08:51 -0800 Subject: intel/fs: Switch to standard vector layout for barycentrics at optimization time. This involves permuting the registers of barycentric vectors to have the standard X[0-n] Y[0-n] layout at NIR translation time. Barycentrics are converted to the format expected by the PLN instruction in the lower_barycentrics() pass run after the optimization loop. Main reason is correctness of SIMD32 fragment shaders. The shuffle_from_pln_layout() and shuffle_to_pln_layout() helpers used during NIR translation are busted for SIMD32. This leads to serious corruption at present with INTEL_DEBUG=do32, especially on Gen11+ where these helpers are hit more frequently due to the lack of a hardware PLN instruction. Of course one could have chosen to fix those helpers instead, but there is another far more subtle issue that was reported during review of the SIMD32 fragment shader codegen changes: The SIMD splitting pass currently handles SIMD32 barycentric vectors as if they had the standard X[0-n] Y[0-n] layout, even though they are interleaved for the PLN instruction, which causes incorrect execution masks to be applied to the MOVs unzipping barycentric vectors in cases where a LINTERP instruction occurs under non-uniform control flow. I'm not aware of any conformance regressions due to the latter issue at present, but for our peace of mind let's move the conversion to the PLN layout into the lower_barycentrics() pass run after lower_simd_width(). This leads to the following shader-db improvements (including SIMD32 shaders) in combination with the previous back-end preparation changes -- Without them (especially the copy propagation changes) this would lead to a massive number of regressions. On ICL: total instructions in shared programs: 20662316 -> 20466903 (-0.95%) instructions in affected programs: 10538474 -> 10343061 (-1.85%) helped: 68775 HURT: 6 total spills in shared programs: 8938 -> 8748 (-2.13%) spills in affected programs: 376 -> 186 (-50.53%) helped: 9 HURT: 5 total fills in shared programs: 8965 -> 8663 (-3.37%) fills in affected programs: 965 -> 663 (-31.30%) helped: 9 HURT: 6 LOST: 146 GAINED: 43 On SKL: total instructions in shared programs: 18725867 -> 18614912 (-0.59%) instructions in affected programs: 3876590 -> 3765635 (-2.86%) helped: 27492 HURT: 2 LOST: 191 GAINED: 417 On SNB: total instructions in shared programs: 14573613 -> 13980646 (-4.07%) instructions in affected programs: 5199074 -> 4606107 (-11.41%) helped: 29998 HURT: 0 LOST: 21 GAINED: 30 Results are somewhat less impressive but still significant without SIMD32 fragment shaders enabled. On ICL: total instructions in shared programs: 16148728 -> 16061659 (-0.54%) instructions in affected programs: 6114788 -> 6027719 (-1.42%) helped: 42046 HURT: 6 total spills in shared programs: 8218 -> 8028 (-2.31%) spills in affected programs: 376 -> 186 (-50.53%) helped: 9 HURT: 5 total fills in shared programs: 8953 -> 8651 (-3.37%) fills in affected programs: 965 -> 663 (-31.30%) helped: 9 HURT: 6 LOST: 0 GAINED: 3 On SKL: total instructions in shared programs: 14927994 -> 14926738 (-0.01%) instructions in affected programs: 168850 -> 167594 (-0.74%) helped: 711 HURT: 2 On SNB: total instructions in shared programs: 10770538 -> 10734403 (-0.34%) instructions in affected programs: 2702172 -> 2666037 (-1.34%) helped: 17818 HURT: 0 All of the hurt shaders are either spilling slightly more or emitting additional NOP instructions due to the SIMD16 POW workaround for Gen8-9 combined with differences in scheduling. Reviewed-by: Kenneth Graunke --- src/intel/compiler/brw_fs.cpp | 15 ++++++++++ src/intel/compiler/brw_fs.h | 5 ++-- src/intel/compiler/brw_fs_nir.cpp | 53 ++++------------------------------- src/intel/compiler/brw_fs_visitor.cpp | 15 +++++----- 4 files changed, 30 insertions(+), 58 deletions(-) (limited to 'src/intel') diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp index f10df1dcbeb..97f47ab92c2 100644 --- a/src/intel/compiler/brw_fs.cpp +++ b/src/intel/compiler/brw_fs.cpp @@ -6766,6 +6766,21 @@ fs_visitor::lower_barycentrics() const fs_builder ubld = ibld.exec_all().group(8, 0); switch (inst->opcode) { + case FS_OPCODE_LINTERP : { + assert(inst->exec_size == 16); + const fs_reg tmp = ibld.vgrf(inst->src[0].type, 2); + fs_reg srcs[4]; + + for (unsigned i = 0; i < ARRAY_SIZE(srcs); i++) + srcs[i] = horiz_offset(offset(inst->src[0], ibld, i % 2), + 8 * (i / 2)); + + ubld.LOAD_PAYLOAD(tmp, srcs, ARRAY_SIZE(srcs), ARRAY_SIZE(srcs)); + + inst->src[0] = tmp; + progress = true; + break; + } case FS_OPCODE_INTERPOLATE_AT_SAMPLE: case FS_OPCODE_INTERPOLATE_AT_SHARED_OFFSET: case FS_OPCODE_INTERPOLATE_AT_PER_SLOT_OFFSET: { diff --git a/src/intel/compiler/brw_fs.h b/src/intel/compiler/brw_fs.h index d84f99db036..a682fac9aa6 100644 --- a/src/intel/compiler/brw_fs.h +++ b/src/intel/compiler/brw_fs.h @@ -571,13 +571,14 @@ namespace brw { return fs_reg(); const fs_reg tmp = bld.vgrf(BRW_REGISTER_TYPE_F, 2); - const brw::fs_builder hbld = bld.exec_all().group(16, 0); + const brw::fs_builder hbld = bld.exec_all().group(8, 0); const unsigned m = bld.dispatch_width() / hbld.dispatch_width(); fs_reg *const components = new fs_reg[2 * m]; for (unsigned c = 0; c < 2; c++) { for (unsigned g = 0; g < m; g++) - components[c * m + g] = offset(brw_vec8_grf(regs[g], 0), hbld, c); + components[c * m + g] = offset(brw_vec8_grf(regs[g / 2], 0), + hbld, c + 2 * (g % 2)); } hbld.LOAD_PAYLOAD(tmp, components, 2 * m, 0); diff --git a/src/intel/compiler/brw_fs_nir.cpp b/src/intel/compiler/brw_fs_nir.cpp index ffaf90764f5..3bed5406576 100644 --- a/src/intel/compiler/brw_fs_nir.cpp +++ b/src/intel/compiler/brw_fs_nir.cpp @@ -3313,44 +3313,6 @@ alloc_frag_output(fs_visitor *v, unsigned location) unreachable("Invalid location"); } -/* Annoyingly, we get the barycentrics into the shader in a layout that's - * optimized for PLN but it doesn't work nearly as well as one would like for - * manual interpolation. - */ -static void -shuffle_from_pln_layout(const fs_builder &bld, fs_reg dest, fs_reg pln_data) -{ - dest.type = BRW_REGISTER_TYPE_F; - pln_data.type = BRW_REGISTER_TYPE_F; - const fs_reg dest_u = offset(dest, bld, 0); - const fs_reg dest_v = offset(dest, bld, 1); - - for (unsigned g = 0; g < bld.dispatch_width() / 8; g++) { - const fs_builder gbld = bld.group(8, g); - gbld.MOV(horiz_offset(dest_u, g * 8), - byte_offset(pln_data, (g * 2 + 0) * REG_SIZE)); - gbld.MOV(horiz_offset(dest_v, g * 8), - byte_offset(pln_data, (g * 2 + 1) * REG_SIZE)); - } -} - -static void -shuffle_to_pln_layout(const fs_builder &bld, fs_reg pln_data, fs_reg src) -{ - pln_data.type = BRW_REGISTER_TYPE_F; - src.type = BRW_REGISTER_TYPE_F; - const fs_reg src_u = offset(src, bld, 0); - const fs_reg src_v = offset(src, bld, 1); - - for (unsigned g = 0; g < bld.dispatch_width() / 8; g++) { - const fs_builder gbld = bld.group(8, g); - gbld.MOV(byte_offset(pln_data, (g * 2 + 0) * REG_SIZE), - horiz_offset(src_u, g * 8)); - gbld.MOV(byte_offset(pln_data, (g * 2 + 1) * REG_SIZE), - horiz_offset(src_v, g * 8)); - } -} - void fs_visitor::nir_emit_fs_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr) @@ -3565,8 +3527,9 @@ fs_visitor::nir_emit_fs_intrinsic(const fs_builder &bld, (enum glsl_interp_mode) nir_intrinsic_interp_mode(instr); enum brw_barycentric_mode bary = brw_barycentric_mode(interp_mode, instr->intrinsic); - - shuffle_from_pln_layout(bld, dest, this->delta_xy[bary]); + const fs_reg srcs[] = { offset(this->delta_xy[bary], bld, 0), + offset(this->delta_xy[bary], bld, 1) }; + bld.LOAD_PAYLOAD(dest, srcs, ARRAY_SIZE(srcs), 0); break; } @@ -3711,18 +3674,12 @@ fs_visitor::nir_emit_fs_intrinsic(const fs_builder &bld, if (bary_intrin == nir_intrinsic_load_barycentric_at_offset || bary_intrin == nir_intrinsic_load_barycentric_at_sample) { - /* Use the result of the PI message. Because the load_barycentric - * intrinsics return a regular vec2 and we need it in PLN layout, we - * have to do a translation. Fortunately, copy-prop cleans this up - * reliably. - */ - dst_xy = bld.vgrf(BRW_REGISTER_TYPE_F, 2); - shuffle_to_pln_layout(bld, dst_xy, get_nir_src(instr->src[0])); + /* Use the result of the PI message. */ + dst_xy = retype(get_nir_src(instr->src[0]), BRW_REGISTER_TYPE_F); } else { /* Use the delta_xy values computed from the payload */ enum brw_barycentric_mode bary = brw_barycentric_mode(interp_mode, bary_intrin); - dst_xy = this->delta_xy[bary]; } diff --git a/src/intel/compiler/brw_fs_visitor.cpp b/src/intel/compiler/brw_fs_visitor.cpp index 476a9c64a5b..81d0e466cc7 100644 --- a/src/intel/compiler/brw_fs_visitor.cpp +++ b/src/intel/compiler/brw_fs_visitor.cpp @@ -176,11 +176,11 @@ fs_visitor::emit_interpolation_setup_gen4() const fs_reg xstart(negate(brw_vec1_grf(1, 0))); const fs_reg ystart(negate(brw_vec1_grf(1, 1))); - if (devinfo->has_pln && dispatch_width == 16) { - for (unsigned i = 0; i < 2; i++) { - abld.half(i).ADD(half(offset(delta_xy, abld, i), 0), + if (devinfo->has_pln) { + for (unsigned i = 0; i < dispatch_width / 8; i++) { + abld.half(i).ADD(half(offset(delta_xy, abld, 0), i), half(this->pixel_x, i), xstart); - abld.half(i).ADD(half(offset(delta_xy, abld, i), 1), + abld.half(i).ADD(half(offset(delta_xy, abld, 1), i), half(this->pixel_y, i), ystart); } } else { @@ -358,11 +358,10 @@ fs_visitor::emit_interpolation_setup_gen6() for (unsigned c = 0; c < 2; c++) { for (unsigned q = 0; q < dispatch_width / 8; q++) { - const unsigned idx = c + (q & 2) + (q & 1) * dispatch_width / 8; set_predicate(BRW_PREDICATE_NORMAL, - bld.half(q).SEL(horiz_offset(delta_xy[i], idx * 8), - horiz_offset(centroid_delta_xy, idx * 8), - horiz_offset(pixel_delta_xy, idx * 8))); + bld.half(q).SEL(half(offset(delta_xy[i], bld, c), q), + half(offset(centroid_delta_xy, bld, c), q), + half(offset(pixel_delta_xy, bld, c), q))); } } } -- cgit v1.2.3