[Mesa-dev] [PATCH v2 03/30] i965/fs: Fix copy propagation of load payload for double operands

[Samuel Iglesias Gonsálvez]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256



On 13/05/16 07:09, Samuel Iglesias Gonsálvez wrote:
> 
> 
> On 13/05/16 01:52, Francisco Jerez wrote:
>> Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:
>> 
>>> From: Iago Toral Quiroga <itoral at igalia.com>
>>> 
>>> Specifically, consider the size of the data type of the operand
>>> to compute the number of registers written.
>>> 
>>> v2 (Sam): - Fix line width (Jordan). - Add an assert (Jordan). 
>>> - Use REG_SIZE in the calculation of regs_written (Curro)
>>> 
>>> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org> 
>>> Reviewed-by: Francisco Jerez <currojerez at riseup.net> --- 
>>> src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 4 +++- 
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>> 
>>> diff --git
>>> a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>>> b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp index
>>> 83791bf..fd423a3 100644 ---
>>> a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp +++
>>> b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp @@
>>> -774,8 +774,10 @@ fs_visitor::opt_copy_propagate_local(void
>>> *copy_prop_ctx, bblock_t *block, inst->dst.file == VGRF) { int
>>> offset = 0; for (int i = 0; i < inst->sources; i++) { +
>>> assert(type_sz(inst->src[i].type) >= 4);
>> 
>> The assertion doesn't look right to me, if you had taken my
>> suggestion into account the code below wouldn't care what the
>> type size is as long as:
>> 
>> | assert(effective_width * type_sz(...) % REG_SIZE == 0);
>> 
> 
> OK, I am going to replace the assert to this one.
> 
>>> int effective_width = i < inst->header_size ? 8 :
>>> inst->exec_size; -            int regs_written =
>>> effective_width / 8; +            int regs_written =
>>> effective_width / 8 * +
>>> type_sz(inst->src[i].type) / REG_SIZE;
>> 
>> Sigh, you left the '/ 8' in there, the expression should have
>> been 'effective_width * type_sz(inst->src[i].type) / REG_SIZE'.
>> 
> 
> Oh, I have not seen this. I will fix it now.
> 
> With these changes, Does it get your R-b?
> 

As this patch has already your R-b, I will keep it.

Thanks,

Sam

> Sam
> 
>>> if (inst->src[i].file == VGRF) { acp_entry *entry =
>>> ralloc(copy_prop_ctx, acp_entry); entry->dst = inst->dst; -- 
>>> 2.5.0
>>> 
>>> _______________________________________________ mesa-dev
>>> mailing list mesa-dev at lists.freedesktop.org 
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXNWOHAAoJEH/0ujLxfcNDFUoQALAMYy5f1ExmcKQGsBP1Dr9x
EDVpGIIspV7TuJzJ/wUQJAwXzdj3ZTATlbjbF05kXgSyJF1kTixGDBM42Ox3Baga
YNIQNSqtXVD5OJExghu7+9AIYWtnNGf527iFCLajl48B1LFsUvML3E3C5WdJgHIg
AGGdwq8Edzh0hOR2wgTmtBBDH9QuFVKi/wsM5jD3iKgDXBbkDw3LIX58Sv28x7YK
PLTZ/MTx8CNnzUKIRtUX3PpmJfAPvflsGpWS6MlSSzHnPP/MtmzVj4gID9zdZe4M
vPZ3sBPKrxW5tWgH5lLnw01SC4JxrrvHo8MiZIX273CJNJbPs72724Zvulu9/AZc
q6ZgvDKuAt1w3GeRL9rKlxrhRBCW0N7dYacZ05TCWQMVN5SBgyC+ZA5u6gUN2CBD
Tjm8eA0NurQn02o1NYy1J6T4s8zi8aC8dq8C3l8aep5Uv0Cp5h3T4uBBWvh9FmMH
5eCcl7DxSL931O1JsKa9353ONjtO0Mp4zZB+0hfffEnX8hd/JomcXJQAEzoAp5fG
/d4BGt1/ANkTF/VlWWQEO3Ckw6Z+s1h6SZQof8e6u/NcthVc4RtzQ84IARfCoIEd
eSZ8SsU0KIBKZIorYiG945yVaZlGe4i2aOGZ1qr1mjJYarIe/Gp5E4gHczrgnyn4
Ag3TDj/z/dXF6xUHbgJg
=x3RO
-----END PGP SIGNATURE-----