Re: [Mesa-dev] [PATCH] intel/fs: Get rid of fs_inst::equals
On Fri, 2019-01-25 at 15:47 -0600, Jason Ekstrand wrote:
> There are piles of fields that it doesn't check so using it is a lie.
> The only reason why it's not causing problem is because it has
> exactly
> one user which only uses it for MOV instructions (which aren't very
> interesting) and only on Sandy Bridge and earlier hardware. Just get
> rid of it and inline it in the one place that it's actually used.
>
> Cc: Iago Toral Quiroga
> ---
> src/intel/compiler/brw_fs.cpp | 29 +++--
> src/intel/compiler/brw_ir_fs.h | 1 -
> 2 files changed, 7 insertions(+), 23 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs.cpp
> b/src/intel/compiler/brw_fs.cpp
> index 0359eb079f7..6b66116f46b 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -212,27 +212,6 @@ fs_visitor::DEP_RESOLVE_MOV(const fs_builder
> &bld, int grf)
> ubld.MOV(ubld.null_reg_f(), fs_reg(VGRF, grf,
> BRW_REGISTER_TYPE_F));
> }
>
> -bool
> -fs_inst::equals(fs_inst *inst) const
> -{
> - return (opcode == inst->opcode &&
> - dst.equals(inst->dst) &&
> - src[0].equals(inst->src[0]) &&
> - src[1].equals(inst->src[1]) &&
> - src[2].equals(inst->src[2]) &&
> - saturate == inst->saturate &&
> - predicate == inst->predicate &&
> - conditional_mod == inst->conditional_mod &&
> - mlen == inst->mlen &&
> - base_mrf == inst->base_mrf &&
> - target == inst->target &&
> - eot == inst->eot &&
> - header_size == inst->header_size &&
> - shadow_compare == inst->shadow_compare &&
> - exec_size == inst->exec_size &&
> - offset == inst->offset);
> -}
> -
> bool
> fs_inst::is_send_from_grf() const
> {
> @@ -3410,7 +3389,13 @@ fs_visitor::remove_duplicate_mrf_writes()
>if (inst->opcode == BRW_OPCODE_MOV &&
> inst->dst.file == MRF) {
> fs_inst *prev_inst = last_mrf_move[inst->dst.nr];
> - if (prev_inst && inst->equals(prev_inst)) {
> + if (prev_inst && prev_inst->opcode == BRW_OPCODE_MOV &&
> + inst->dst.equals(prev_inst->dst) &&
> + inst->src[0].equals(prev_inst->src[0]) &&
Wrong indentation. Otherwise:
Reviewed-by: Iago Toral Quiroga
> + inst->saturate == prev_inst->saturate &&
> + inst->predicate == prev_inst->predicate &&
> + inst->conditional_mod == prev_inst->conditional_mod &&
> + inst->exec_size == prev_inst->exec_size) {
> inst->remove(block);
> progress = true;
> continue;
> diff --git a/src/intel/compiler/brw_ir_fs.h
> b/src/intel/compiler/brw_ir_fs.h
> index 08e3d83d910..d05357e822e 100644
> --- a/src/intel/compiler/brw_ir_fs.h
> +++ b/src/intel/compiler/brw_ir_fs.h
> @@ -347,7 +347,6 @@ public:
>
> void resize_sources(uint8_t num_sources);
>
> - bool equals(fs_inst *inst) const;
> bool is_send_from_grf() const;
> bool is_partial_write() const;
> bool is_copy_payload(const brw::simple_allocator &grf_alloc)
> const;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/fs: Get rid of fs_inst::equals
There are piles of fields that it doesn't check so using it is a lie.
The only reason why it's not causing problem is because it has exactly
one user which only uses it for MOV instructions (which aren't very
interesting) and only on Sandy Bridge and earlier hardware. Just get
rid of it and inline it in the one place that it's actually used.
Cc: Iago Toral Quiroga
---
src/intel/compiler/brw_fs.cpp | 29 +++--
src/intel/compiler/brw_ir_fs.h | 1 -
2 files changed, 7 insertions(+), 23 deletions(-)
diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index 0359eb079f7..6b66116f46b 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/src/intel/compiler/brw_fs.cpp
@@ -212,27 +212,6 @@ fs_visitor::DEP_RESOLVE_MOV(const fs_builder &bld, int grf)
ubld.MOV(ubld.null_reg_f(), fs_reg(VGRF, grf, BRW_REGISTER_TYPE_F));
}
-bool
-fs_inst::equals(fs_inst *inst) const
-{
- return (opcode == inst->opcode &&
- dst.equals(inst->dst) &&
- src[0].equals(inst->src[0]) &&
- src[1].equals(inst->src[1]) &&
- src[2].equals(inst->src[2]) &&
- saturate == inst->saturate &&
- predicate == inst->predicate &&
- conditional_mod == inst->conditional_mod &&
- mlen == inst->mlen &&
- base_mrf == inst->base_mrf &&
- target == inst->target &&
- eot == inst->eot &&
- header_size == inst->header_size &&
- shadow_compare == inst->shadow_compare &&
- exec_size == inst->exec_size &&
- offset == inst->offset);
-}
-
bool
fs_inst::is_send_from_grf() const
{
@@ -3410,7 +3389,13 @@ fs_visitor::remove_duplicate_mrf_writes()
if (inst->opcode == BRW_OPCODE_MOV &&
inst->dst.file == MRF) {
fs_inst *prev_inst = last_mrf_move[inst->dst.nr];
-if (prev_inst && inst->equals(prev_inst)) {
+if (prev_inst && prev_inst->opcode == BRW_OPCODE_MOV &&
+ inst->dst.equals(prev_inst->dst) &&
+inst->src[0].equals(prev_inst->src[0]) &&
+ inst->saturate == prev_inst->saturate &&
+ inst->predicate == prev_inst->predicate &&
+ inst->conditional_mod == prev_inst->conditional_mod &&
+ inst->exec_size == prev_inst->exec_size) {
inst->remove(block);
progress = true;
continue;
diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h
index 08e3d83d910..d05357e822e 100644
--- a/src/intel/compiler/brw_ir_fs.h
+++ b/src/intel/compiler/brw_ir_fs.h
@@ -347,7 +347,6 @@ public:
void resize_sources(uint8_t num_sources);
- bool equals(fs_inst *inst) const;
bool is_send_from_grf() const;
bool is_partial_write() const;
bool is_copy_payload(const brw::simple_allocator &grf_alloc) const;
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/fs: Get rid of fs_inst::equals
There are piles of fields that it doesn't check so using it is a lie.
The only reason why it's not causing problem is because it has exactly
one user which only uses it for MOV instructions (which aren't very
interesting) and only on Sandy Bridge and earlier hardware. Just get
rid of it and inline it in the one place that it's actually used.
Cc: Iago Toral Quiroga
---
src/intel/compiler/brw_fs.cpp | 29 +++--
src/intel/compiler/brw_ir_fs.h | 1 -
2 files changed, 7 insertions(+), 23 deletions(-)
diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index 0359eb079f7..6b66116f46b 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/src/intel/compiler/brw_fs.cpp
@@ -212,27 +212,6 @@ fs_visitor::DEP_RESOLVE_MOV(const fs_builder &bld, int grf)
ubld.MOV(ubld.null_reg_f(), fs_reg(VGRF, grf, BRW_REGISTER_TYPE_F));
}
-bool
-fs_inst::equals(fs_inst *inst) const
-{
- return (opcode == inst->opcode &&
- dst.equals(inst->dst) &&
- src[0].equals(inst->src[0]) &&
- src[1].equals(inst->src[1]) &&
- src[2].equals(inst->src[2]) &&
- saturate == inst->saturate &&
- predicate == inst->predicate &&
- conditional_mod == inst->conditional_mod &&
- mlen == inst->mlen &&
- base_mrf == inst->base_mrf &&
- target == inst->target &&
- eot == inst->eot &&
- header_size == inst->header_size &&
- shadow_compare == inst->shadow_compare &&
- exec_size == inst->exec_size &&
- offset == inst->offset);
-}
-
bool
fs_inst::is_send_from_grf() const
{
@@ -3410,7 +3389,13 @@ fs_visitor::remove_duplicate_mrf_writes()
if (inst->opcode == BRW_OPCODE_MOV &&
inst->dst.file == MRF) {
fs_inst *prev_inst = last_mrf_move[inst->dst.nr];
-if (prev_inst && inst->equals(prev_inst)) {
+if (prev_inst && prev_inst->opcode == BRW_OPCODE_MOV &&
+ inst->dst.equals(prev_inst->dst) &&
+inst->src[0].equals(prev_inst->src[0]) &&
+ inst->saturate == prev_inst->saturate &&
+ inst->predicate == prev_inst->predicate &&
+ inst->conditional_mod == prev_inst->conditional_mod &&
+ inst->exec_size == prev_inst->exec_size) {
inst->remove(block);
progress = true;
continue;
diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h
index 08e3d83d910..d05357e822e 100644
--- a/src/intel/compiler/brw_ir_fs.h
+++ b/src/intel/compiler/brw_ir_fs.h
@@ -347,7 +347,6 @@ public:
void resize_sources(uint8_t num_sources);
- bool equals(fs_inst *inst) const;
bool is_send_from_grf() const;
bool is_partial_write() const;
bool is_copy_payload(const brw::simple_allocator &grf_alloc) const;
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
