<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 26, 2016 at 8:46 PM, Francisco Jerez <<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The Gen5+ sampler message payload construction code steps through the coordinate and derivative components by induction like 'coordinate = offset(coordinate, bld, 1)', the problem is that while doing that it may step one past the end of the coordinate vector causing an assertion failure in offset() if it happens to be a (single component) immediate. Right now coordinates and derivatives are typically passed as actual registers but that will no longer be the case when we start propagating constants into logical messages. Instead express coordinate components in closed form like 'offset(coordinate, bld, i)' -- The end result seems slightly more readable that way and it allows passing the coordinate and derivative registers by const reference instead of by value, so it seems like a clean-up in its own right. </blockquote><div> </div><div>Thank you!!! This is way better than the way it was before. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> --- src/mesa/drivers/dri/i965/brw_fs.cpp | 62 +++++++++++++++--------------------- 1 file changed, 25 insertions(+), 37 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 31ebbe3..e05977b 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -4022,9 +4022,9 @@ lower_sampler_logical_send_gen4(const fs_builder &bld, fs_inst *inst, opcode op, static void lower_sampler_logical_send_gen5(const fs_builder &bld, fs_inst *inst, opcode op, - fs_reg coordinate, + const fs_reg &coordinate, const fs_reg &shadow_c, - fs_reg lod, fs_reg lod2, + const fs_reg &lod, const fs_reg &lod2, const fs_reg &sample_index, const fs_reg &surface, const fs_reg &sampler, @@ -4044,10 +4044,10 @@ lower_sampler_logical_send_gen5(const fs_builder &bld, fs_inst *inst, opcode op, message.nr--; } - for (unsigned i = 0; i < coord_components; i++) { - bld.MOV(retype(offset(msg_coords, bld, i), coordinate.type), coordinate); - coordinate = offset(coordinate, bld, 1); - } + for (unsigned i = 0; i < coord_components; i++) + bld.MOV(retype(offset(msg_coords, bld, i), coordinate.type), + offset(coordinate, bld, i)); + fs_reg msg_end = offset(msg_coords, bld, coord_components); fs_reg msg_lod = offset(msg_coords, bld, 4); @@ -4076,12 +4076,10 @@ lower_sampler_logical_send_gen5(const fs_builder &bld, fs_inst *inst, opcode op, */ msg_end = msg_lod; for (unsigned i = 0; i < grad_components; i++) { - bld.MOV(msg_end, lod); - lod = offset(lod, bld, 1); + bld.MOV(msg_end, offset(lod, bld, i)); msg_end = offset(msg_end, bld, 1); - bld.MOV(msg_end, lod2); - lod2 = offset(lod2, bld, 1); + bld.MOV(msg_end, offset(lod2, bld, i)); msg_end = offset(msg_end, bld, 1); } break; @@ -4131,14 +4129,14 @@ is_high_sampler(const struct brw_device_info *devinfo, const fs_reg &sampler) static void lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op, - fs_reg coordinate, + const fs_reg &coordinate, const fs_reg &shadow_c, - fs_reg lod, fs_reg lod2, + fs_reg lod, const fs_reg &lod2, const fs_reg &sample_index, const fs_reg &mcs, const fs_reg &surface, const fs_reg &sampler, - fs_reg offset_value, + const fs_reg &offset_value, unsigned coord_components, unsigned grad_components) { @@ -4210,21 +4208,15 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op, * [hdr], [ref], x, dPdx.x, dPdy.x, y, dPdx.y, dPdy.y, z, dPdx.z, dPdy.z */ for (unsigned i = 0; i < coord_components; i++) { - bld.MOV(sources[length], coordinate); - coordinate = offset(coordinate, bld, 1); + bld.MOV(sources[length], offset(coordinate, bld, i)); length++; /* For cube map array, the coordinate is (u,v,r,ai) but there are * only derivatives for (u, v, r). */ if (i < grad_components) { - bld.MOV(sources[length], lod); - lod = offset(lod, bld, 1); - length++; - - bld.MOV(sources[length], lod2); - lod2 = offset(lod2, bld, 1); - length++; + bld.MOV(sources[length++], offset(lod, bld, i)); + bld.MOV(sources[length++], offset(lod2, bld, i)); } } @@ -4239,13 +4231,12 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op, * On Gen9 they are u, v, lod, r */ bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate); - coordinate = offset(coordinate, bld, 1); length++; if (devinfo->gen >= 9) { if (coord_components >= 2) { - bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate); - coordinate = offset(coordinate, bld, 1); + bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), + offset(coordinate, bld, 1)); } length++; } @@ -4254,8 +4245,8 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op, length++; for (unsigned i = devinfo->gen >= 9 ? 2 : 1; i < coord_components; i++) { - bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate); - coordinate = offset(coordinate, bld, 1); + bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), + offset(coordinate, bld, i)); length++; } @@ -4293,8 +4284,8 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op, * texture coordinates. */ for (unsigned i = 0; i < coord_components; i++) { - bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), coordinate); - coordinate = offset(coordinate, bld, 1); + bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), + offset(coordinate, bld, i)); length++; } @@ -4306,20 +4297,18 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op, /* More crazy intermixing */ for (unsigned i = 0; i < 2; i++) { /* u, v */ - bld.MOV(sources[length], coordinate); - coordinate = offset(coordinate, bld, 1); + bld.MOV(sources[length], offset(coordinate, bld, i)); length++; </blockquote><div> </div><div>Might be good to build the length++ into the bld.MOV while you're here. Same for the ones below. If you don't want the churn, I can write a follow-on easily enough. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> } for (unsigned i = 0; i < 2; i++) { /* offu, offv */ - bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), offset_value); - offset_value = offset(offset_value, bld, 1); + bld.MOV(retype(sources[length], BRW_REGISTER_TYPE_D), + offset(offset_value, bld, i)); length++; } if (coord_components == 3) { /* r if present */ - bld.MOV(sources[length], coordinate); - coordinate = offset(coordinate, bld, 1); + bld.MOV(sources[length], offset(coordinate, bld, 2)); length++; } @@ -4332,8 +4321,7 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op, /* Set up the coordinate (except for cases where it was done above) */ if (!coordinate_done) { for (unsigned i = 0; i < coord_components; i++) { - bld.MOV(sources[length], coordinate); - coordinate = offset(coordinate, bld, 1); + bld.MOV(sources[length], offset(coordinate, bld, i)); length++; } } -- 2.7.3 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>