On 04.06.2014 15:22, Matt Turner wrote:
On Tue, Jun 3, 2014 at 3:59 PM, Abdiel Janulgue
<abdiel.janul...@linux.intel.com> wrote:
The negation source modifier on src registers has changed meaning in Broadwell
when
used with logical operations.
Make sure copy propagation occurs only for original statements that does not
have
negated source registers and destination instruction is not a logical op. In
addition,
since we have added 'NOT' as a potentially propagate-able instruction, don't
propagate it either when the destination instruction is not a logical op.
Signed-off-by: Abdiel Janulgue <abdiel.janul...@linux.intel.com>
---
src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 14 ++++++++++++++
1 file changed, 14 insertions(+)
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 09d5949..26eda92 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
@@ -273,6 +273,15 @@ fs_copy_prop_dataflow::dump_block_data() const
}
}
+static bool
+is_logic_op(enum opcode opcode)
+{
+ return (opcode == BRW_OPCODE_AND ||
+ opcode == BRW_OPCODE_OR ||
+ opcode == BRW_OPCODE_XOR ||
+ opcode == BRW_OPCODE_NOT);
+}
+
bool
fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
{
@@ -331,6 +340,11 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg,
acp_entry *entry)
if (has_source_modifiers && entry->dst.type != inst->src[arg].type)
return false;
+ if (brw->gen >=8 &&
+ ((entry->src.negate && is_logic_op(inst->opcode)) ||
+ (entry->opcode == BRW_OPCODE_NOT && !is_logic_op(inst->opcode))))
+ return false;
The static function and some bit of this logic need to go into the
previous patch. (And some bits, like adding opcode to the entry needs
to go in this patch)
This conditional doesn't allow copy propagation from
not a, -b
and c, d, a
since it return false if gen >= 8 && is_logic_op(inst) &&
entry->src.negate (which in the case of a NOT, means ~, not negation).
It's a trivial case, but fixing it should entail splitting the
conditional and making it clearer.
Maybe we want something like:
if (brw->gen >= 8) {
if (entry->opcode == BRW_OPCODE_NOT) {
if (!is_logic_op(inst->opcode)) {
return false;
}
} else if (entry->src.negate) {
if (is_logic_op(inst->opcode) {
return false;
}
}
}
That makes it more clear. Thanks for the feedback. I'll have the v2
coming up soon.
That lets it handle NOT a, -b properly.
_______________________________________________
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
