Mesa (staging/18.2): intel: Don't propagate conditional modifiers if a UD source is negated

[GitLab Mirror]

Module: Mesa
Branch: staging/18.2
Commit: a0782c61b27b865c438be4e4513a06cf1eb16d60
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=a0782c61b27b865c438be4e4513a06cf1eb16d60

Author: Jason Ekstrand <jason.ekstrand at intel.com>
Date:   Mon Oct  8 12:22:35 2018 -0500

intel: Don't propagate conditional modifiers if a UD source is negated

This fixes a bug uncovered by my NIR integer division by constant
optimization series.

Fixes: 19f9cb72c8b "i965/fs: Add pass to propagate conditional..."
Fixes: 627f94b72e0 "i965/vec4: adding vec4_cmod_propagation..."
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
(cherry picked from commit 4ba445e0117b29c31b030feb6e0f421a5ceb03e5)

---

 src/intel/compiler/brw_fs.cpp    | 19 +++++++++++++++++++
 src/intel/compiler/brw_ir_fs.h   |  1 +
 src/intel/compiler/brw_ir_vec4.h |  1 +
 src/intel/compiler/brw_reg.h     |  9 +++++++++
 src/intel/compiler/brw_vec4.cpp  | 20 ++++++++++++++++++++
 5 files changed, 50 insertions(+)

diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index 5b87991652..1183e7c898 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/src/intel/compiler/brw_fs.cpp
@@ -394,6 +394,25 @@ fs_inst::can_do_source_mods(const struct gen_device_info *devinfo)
 }
 
 bool
+fs_inst::can_do_cmod()
+{
+   if (!backend_instruction::can_do_cmod())
+      return false;
+
+   /* The accumulator result appears to get used for the conditional modifier
+    * generation.  When negating a UD value, there is a 33rd bit generated for
+    * the sign in the accumulator value, so now you can't check, for example,
+    * equality with a 32-bit value.  See piglit fs-op-neg-uvec4.
+    */
+   for (unsigned i = 0; i < sources; i++) {
+      if (type_is_unsigned_int(src[i].type) && src[i].negate)
+         return false;
+   }
+
+   return true;
+}
+
+bool
 fs_inst::can_change_types() const
 {
    return dst.type == src[0].type &&
diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h
index 92dad269a3..07e7224e0f 100644
--- a/src/intel/compiler/brw_ir_fs.h
+++ b/src/intel/compiler/brw_ir_fs.h
@@ -354,6 +354,7 @@ public:
    unsigned components_read(unsigned i) const;
    unsigned size_read(int arg) const;
    bool can_do_source_mods(const struct gen_device_info *devinfo);
+   bool can_do_cmod();
    bool can_change_types() const;
    bool has_source_and_destination_hazard() const;
 
diff --git a/src/intel/compiler/brw_ir_vec4.h b/src/intel/compiler/brw_ir_vec4.h
index e401d8b4d1..65b1e4f3b5 100644
--- a/src/intel/compiler/brw_ir_vec4.h
+++ b/src/intel/compiler/brw_ir_vec4.h
@@ -291,6 +291,7 @@ public:
                       int swizzle, int swizzle_mask);
    void reswizzle(int dst_writemask, int swizzle);
    bool can_do_source_mods(const struct gen_device_info *devinfo);
+   bool can_do_cmod();
    bool can_do_writemask(const struct gen_device_info *devinfo);
    bool can_change_types() const;
    bool has_source_and_destination_hazard() const;
diff --git a/src/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
index ac12ab3d2d..46d66198a1 100644
--- a/src/intel/compiler/brw_reg.h
+++ b/src/intel/compiler/brw_reg.h
@@ -376,6 +376,15 @@ brw_int_type(unsigned sz, bool is_signed)
    }
 }
 
+static inline bool
+type_is_unsigned_int(enum brw_reg_type tp)
+{
+   return tp == BRW_REGISTER_TYPE_UB ||
+          tp == BRW_REGISTER_TYPE_UW ||
+          tp == BRW_REGISTER_TYPE_UD ||
+          tp == BRW_REGISTER_TYPE_UQ;
+}
+
 /**
  * Construct a brw_reg.
  * \param file      one of the BRW_x_REGISTER_FILE values
diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp
index 4e242e0303..e2fa58502f 100644
--- a/src/intel/compiler/brw_vec4.cpp
+++ b/src/intel/compiler/brw_vec4.cpp
@@ -257,6 +257,26 @@ vec4_instruction::can_do_source_mods(const struct gen_device_info *devinfo)
 }
 
 bool
+vec4_instruction::can_do_cmod()
+{
+   if (!backend_instruction::can_do_cmod())
+      return false;
+
+   /* The accumulator result appears to get used for the conditional modifier
+    * generation.  When negating a UD value, there is a 33rd bit generated for
+    * the sign in the accumulator value, so now you can't check, for example,
+    * equality with a 32-bit value.  See piglit fs-op-neg-uvec4.
+    */
+   for (unsigned i = 0; i < 3; i++) {
+      if (src[i].file != BAD_FILE &&
+          type_is_unsigned_int(src[i].type) && src[i].negate)
+         return false;
+   }
+
+   return true;
+}
+
+bool
 vec4_instruction::can_do_writemask(const struct gen_device_info *devinfo)
 {
    switch (opcode) {