[Mesa-dev,12/32] i965/fs: Fix lower_load_payload() to take into account stride in the metadata guess.

Submitted by Francisco Jerez on Feb. 6, 2015, 2:42 p.m.

Details

Message ID 1423233792-11767-12-git-send-email-currojerez@riseup.net
State New
Headers show

Not browsing as part of any series.

Commit Message

Francisco Jerez Feb. 6, 2015, 2:42 p.m.
---
 src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 163aa41..8da1f47 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -3074,7 +3074,7 @@  fs_visitor::lower_load_payload()
          bool force_sechalf = inst->force_sechalf &&
                               !inst->force_writemask_all;
          bool toggle_sechalf = inst->dst.width == 16 &&
-                               type_sz(inst->dst.type) == 4 &&
+                               type_sz(inst->dst.type) * inst->dst.stride == 4 &&
                                !inst->force_writemask_all;
          for (int i = 0; i < inst->regs_written; ++i) {
             metadata[dst_reg + i].written = true;

Comments

Where are you getting a destination stride in a load_payload?  The whole
point of load_payload was to remove partial writes so this seems to go
against everything it's intended for.
--Jason

On Fri, Feb 6, 2015 at 9:42 AM, Francisco Jerez <currojerez@riseup.net>
wrote:

> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 163aa41..8da1f47 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -3074,7 +3074,7 @@ fs_visitor::lower_load_payload()
>           bool force_sechalf = inst->force_sechalf &&
>                                !inst->force_writemask_all;
>           bool toggle_sechalf = inst->dst.width == 16 &&
> -                               type_sz(inst->dst.type) == 4 &&
> +                               type_sz(inst->dst.type) * inst->dst.stride
> == 4 &&
>                                 !inst->force_writemask_all;
>           for (int i = 0; i < inst->regs_written; ++i) {
>              metadata[dst_reg + i].written = true;
> --
> 2.1.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
Jason Ekstrand <jason@jlekstrand.net> writes:

> Where are you getting a destination stride in a load_payload?  The whole
> point of load_payload was to remove partial writes so this seems to go
> against everything it's intended for.
> --Jason

Yeah, maybe...  This case was triggered by some of my image_load_store
code that ends up using byte types with stride=4 for some image formats.

>
> On Fri, Feb 6, 2015 at 9:42 AM, Francisco Jerez <currojerez@riseup.net>
> wrote:
>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 163aa41..8da1f47 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -3074,7 +3074,7 @@ fs_visitor::lower_load_payload()
>>           bool force_sechalf = inst->force_sechalf &&
>>                                !inst->force_writemask_all;
>>           bool toggle_sechalf = inst->dst.width == 16 &&
>> -                               type_sz(inst->dst.type) == 4 &&
>> +                               type_sz(inst->dst.type) * inst->dst.stride
>> == 4 &&
>>                                 !inst->force_writemask_all;
>>           for (int i = 0; i < inst->regs_written; ++i) {
>>              metadata[dst_reg + i].written = true;
>> --
>> 2.1.3
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>