On Wed, Sep 16, 2015 at 12:47 PM, Alejandro Piñeiro
<apinhe...@igalia.com> wrote:
> SEL and MOV instructions, as long as they don't have source modifiers, are
> just copying bits around. So those kind of instruction could be propagated
> even if there are type mismatches. This is needed because NIR generates
> integer SEL and MOV instructions whenever it doesn't know what else to
> generate.
>
> This commit adds support for copy propagation using previous instruction
> as reference.
> ---
>
> I was tempted to try to remove copy_entry->value, as with this commit
> we are tracking the instructions too, but I think that the code would
> be clearer this way.
>
>
> .../drivers/dri/i965/brw_vec4_copy_propagation.cpp | 35
> +++++++++++++++-------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> index 64e2528..f8ecd0b 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> @@ -39,6 +39,7 @@ namespace brw {
>
> struct copy_entry {
> src_reg *value[4];
> + vec4_instruction *inst[4];
> int saturatemask;
> };
>
> @@ -320,7 +321,8 @@ try_copy_propagate(const struct brw_device_info *devinfo,
>
> if (has_source_modifiers &&
> value.type != inst->src[arg].type &&
> - !can_change_source_types(inst))
> + !can_change_source_types(inst) &&
> + !can_change_source_types(entry->inst[arg]))
This isn't right. The entry->inst array is indexed on vec4 component
but arg is the argument of the instruction. I think what you want to
do is add something to the loop above to loop over 0...3 and check
them all. Also, how this is different from has_source_modifiers?
Obviously, it is (the shader-db numbers say so) but I'm not seeing it.
Could you please provide an example.
> return false;
>
> if (has_source_modifiers &&
> @@ -338,7 +340,8 @@ try_copy_propagate(const struct brw_device_info *devinfo,
> * instead. See also resolve_ud_negate().
> */
> if (value.negate &&
> - value.type == BRW_REGISTER_TYPE_UD)
> + value.type == BRW_REGISTER_TYPE_UD &&
> + !can_change_source_types(entry->inst[arg]))
> return false;
>
> /* Don't report progress if this is a noop. */
> @@ -376,17 +379,25 @@ try_copy_propagate(const struct brw_device_info
> *devinfo,
>
> if (has_source_modifiers &&
> value.type != inst->src[arg].type) {
> - /* We are propagating source modifiers from a MOV with a different
> - * type. If we got here, then we can just change the source and
> - * destination types of the instruction and keep going.
> + /* We are propagating source modifiers from a safe instruction with a
> + * different type. If we got here, then we can just change the source
> + * and destination types of the current instruction or the instruction
> + * from we are propagating.
> */
> - assert(can_change_source_types(inst));
> - for (int i = 0; i < 3; i++) {
> - inst->src[i].type = value.type;
> + assert(can_change_source_types(inst) ||
> + can_change_source_types(entry->inst[arg]));
> +
> + if (can_change_source_types(inst)) {
> + for (int i = 0; i < 3; i++) {
> + inst->src[i].type = value.type;
> + }
> + inst->dst.type = value.type;
> + } else {
> + value.type = inst->src[arg].type;
> }
> - inst->dst.type = value.type;
> - } else
> + } else {
> value.type = inst->src[arg].type;
> + }
> inst->src[arg] = value;
> return true;
> }
> @@ -439,7 +450,7 @@ vec4_visitor::opt_copy_propagation(bool do_constant_prop)
> for (c = 0; c < 4; c++) {
> int channel = BRW_GET_SWZ(inst->src[i].swizzle, c);
> entry.value[c] = entries[reg].value[channel];
> -
> + entry.inst[c] = entries[reg].inst[channel];
> /* If there's no available copy for this channel, bail.
> * We could be more aggressive here -- some channels might
> * not get used based on the destination writemask.
> @@ -484,6 +495,7 @@ vec4_visitor::opt_copy_propagation(bool do_constant_prop)
> entries[reg].value[i] = direct_copy ? &inst->src[0] : NULL;
> entries[reg].saturatemask |=
> inst->saturate && direct_copy ? 1 << i : 0;
> + entries[reg].inst[i] = direct_copy ? inst : NULL;
> }
> }
>
> @@ -498,6 +510,7 @@ vec4_visitor::opt_copy_propagation(bool do_constant_prop)
> if (is_channel_updated(inst, entries[i].value, j)) {
> entries[i].value[j] = NULL;
> entries[i].saturatemask &= ~(1 << j);
> + entries[i].inst[j] = NULL;
> }
> }
> }
> --
> 2.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
