Re: [Mesa-dev] [PATCH 5/7] i965/fs: Less broken handling of force_writemask_all in lower_load_payload().

2015-02-20 Thread Jason Ekstrand 
Reviewed-by: Jason Ekstrand jason.ekstr...@intel.com

On Sat, Jan 17, 2015 at 6:04 PM, Francisco Jerez curroje...@riseup.net
wrote:

 It's perfectly fine to read the second half of a register written with
 force_writemask_all from a first half MOV instruction or vice versa, and
 lower_load_payload shouldn't mark the whole MOV as belonging to the second
 half in that case.  Replicate the same metadata to both halves of the
 destination when writemasking is disabled.
 ---
  src/mesa/drivers/dri/i965/brw_fs.cpp | 20 +---
  1 file changed, 13 insertions(+), 7 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
 b/src/mesa/drivers/dri/i965/brw_fs.cpp
 index 4a61943..d585a67 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
 @@ -3059,9 +3059,11 @@ fs_visitor::lower_load_payload()
}

if (inst-dst.file == MRF || inst-dst.file == GRF) {
 - bool force_sechalf = inst-force_sechalf;
 + 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) == 4 
 +   !inst-force_writemask_all;
   for (int i = 0; i  inst-regs_written; ++i) {
  metadata[dst_reg + i].written = true;
  metadata[dst_reg + i].force_sechalf = force_sechalf;
 @@ -3104,11 +3106,15 @@ fs_visitor::lower_load_payload()
mov-force_writemask_all =
 metadata[src_reg].force_writemask_all;
metadata[dst_reg] = metadata[src_reg];
if (dst.width * type_sz(dst.type)  32) {
 - assert((!metadata[src_reg].written ||
 - !metadata[src_reg].force_sechalf) 
 -(!metadata[src_reg + 1].written ||
 - metadata[src_reg + 1].force_sechalf));
 - metadata[dst_reg + 1] = metadata[src_reg + 1];
 + if (metadata[src_reg].force_writemask_all) {
 +metadata[dst_reg + 1] = metadata[src_reg];
 + } else {
 +assert((!metadata[src_reg].written ||
 +!metadata[src_reg].force_sechalf) 
 +   (!metadata[src_reg + 1].written ||
 +metadata[src_reg + 1].force_sechalf));
 +metadata[dst_reg + 1] = metadata[src_reg + 1];
 + }
}
 } else {
metadata[dst_reg].force_writemask_all = false;
 --
 2.1.3

 ___
 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

[Mesa-dev] [PATCH 5/7] i965/fs: Less broken handling of force_writemask_all in lower_load_payload().

2015-01-17 Thread Francisco Jerez 
It's perfectly fine to read the second half of a register written with
force_writemask_all from a first half MOV instruction or vice versa, and
lower_load_payload shouldn't mark the whole MOV as belonging to the second
half in that case.  Replicate the same metadata to both halves of the
destination when writemasking is disabled.
---
 src/mesa/drivers/dri/i965/brw_fs.cpp | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 4a61943..d585a67 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -3059,9 +3059,11 @@ fs_visitor::lower_load_payload()
   }
 
   if (inst-dst.file == MRF || inst-dst.file == GRF) {
- bool force_sechalf = inst-force_sechalf;
+ 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) == 4 
+   !inst-force_writemask_all;
  for (int i = 0; i  inst-regs_written; ++i) {
 metadata[dst_reg + i].written = true;
 metadata[dst_reg + i].force_sechalf = force_sechalf;
@@ -3104,11 +3106,15 @@ fs_visitor::lower_load_payload()
   mov-force_writemask_all = 
metadata[src_reg].force_writemask_all;
   metadata[dst_reg] = metadata[src_reg];
   if (dst.width * type_sz(dst.type)  32) {
- assert((!metadata[src_reg].written ||
- !metadata[src_reg].force_sechalf) 
-(!metadata[src_reg + 1].written ||
- metadata[src_reg + 1].force_sechalf));
- metadata[dst_reg + 1] = metadata[src_reg + 1];
+ if (metadata[src_reg].force_writemask_all) {
+metadata[dst_reg + 1] = metadata[src_reg];
+ } else {
+assert((!metadata[src_reg].written ||
+!metadata[src_reg].force_sechalf) 
+   (!metadata[src_reg + 1].written ||
+metadata[src_reg + 1].force_sechalf));
+metadata[dst_reg + 1] = metadata[src_reg + 1];
+ }
   }
} else {
   metadata[dst_reg].force_writemask_all = false;
-- 
2.1.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev