[Mesa-dev] [PATCH 08/15] i965/fs: Add fs_inst copy constructor.

Pohjolainen, Topi topi.pohjolainen at intel.com
Tue Oct 29 10:32:46 CET 2013


On Mon, Oct 28, 2013 at 11:31:32AM -0700, Matt Turner wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 30 ++++++++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_fs.h   |  1 +
>  2 files changed, 31 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index b2eac6c..28d369a 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -71,6 +71,36 @@ fs_inst::fs_inst()
>     init();
>  }
>  
> +fs_inst::fs_inst(const fs_inst *that)

Is there a particular reason why you chose to introduce this as a conversion
constructor instead of as a proper copy? I'm just afraid that it allows the
compiler to accept, for example, the following (which is probably not what the
author wanted):

void print(const fs_inst& a)
{
}

void foo(const fs_inst *a)
{
	const fs_inst& b = a; // makes a copy of 'a'

	print(a); // also makes a copy of 'a'
}

> +{
> +   memset(this, 0, sizeof(*this));
> +   this->opcode = that->opcode;
> +   this->predicate = that->predicate;
> +   this->predicate_inverse = that->predicate_inverse;
> +
> +   this->dst = that->dst;
> +   this->src[0] = that->src[0];
> +   this->src[1] = that->src[1];
> +   this->src[2] = that->src[2];
> +   this->saturate = that->saturate;
> +   this->conditional_mod = that->conditional_mod;
> +   this->flag_subreg = that->flag_subreg;
> +
> +   this->mlen = that->mlen;
> +   this->regs_written = that->regs_written;
> +   this->base_mrf = that->base_mrf;
> +   this->texture_offset = that->texture_offset;
> +   this->sampler = that->sampler;
> +   this->target = that->target;
> +   this->eot = that->eot;
> +   this->header_present = that->header_present;
> +   this->shadow_compare = that->shadow_compare;
> +   this->force_uncompressed = that->force_uncompressed;
> +   this->force_sechalf = that->force_sechalf;
> +   this->force_writemask_all = that->force_writemask_all;
> +   this->offset = that->offset;
> +}
> +
>  fs_inst::fs_inst(enum opcode opcode)
>  {
>     init();
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index d6f9c9a..dff6ec1 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -136,6 +136,7 @@ public:
>     void init();
>  
>     fs_inst();
> +   fs_inst(const fs_inst *that);
>     fs_inst(enum opcode opcode);
>     fs_inst(enum opcode opcode, fs_reg dst);
>     fs_inst(enum opcode opcode, fs_reg dst, fs_reg src0);
> -- 
> 1.8.3.2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list