Re: [Mesa-dev] [PATCH 2/2] freedreno: ir3: fix wrong return if reg is an array
On 10/24/18 11:05 PM, Rob Clark wrote:
On Tue, Oct 23, 2018 at 9:57 PM Hyunjun Ko wrote:
Since ir3_register struct has union, it could return true even
if it's an array register accidentally when checking whether it
is address/predicate register.
Fixes: dEQP-GLES31.functional.ssbo.layout.random.arrays_of_arrays.6
---
src/gallium/drivers/freedreno/ir3/ir3.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/gallium/drivers/freedreno/ir3/ir3.h
b/src/gallium/drivers/freedreno/ir3/ir3.h
index 3055c10f1d..db94603558 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3.h
+++ b/src/gallium/drivers/freedreno/ir3/ir3.h
@@ -739,7 +739,7 @@ static inline bool writes_addr(struct ir3_instruction
*instr)
{
if (instr->regs_count > 0) {
struct ir3_register *dst = instr->regs[0];
- return reg_num(dst) == REG_A0;
+ return (reg_num(dst) == REG_A0) && !(dst->flags &
IR3_REG_ARRAY);
hmm, good catch.. although I wonder if writes_gpr() in ir3_ra.c has
the same issue. Or anywhere else? (Otoh, I guess checking for CONST
and IMMED in writes_gpr() is a bit silly)
From a quick look at the IR3_REG_* flags, I think IR3_REG_ARRAY is the
only problematic case.. but this is a bit more fragile than it should
be. Maybe we should just move 'int num' out of the union? Maybe that
was too much premature optimization?
BR,
-R
Didn't figure out writes_gpr() might have same issue.
I think taking num out of union should avoid other similar issues if we
don't think
that would increase memory footprint that much.
Working on this.
}
return false;
}
@@ -748,7 +748,7 @@ static inline bool writes_pred(struct ir3_instruction
*instr)
{
if (instr->regs_count > 0) {
struct ir3_register *dst = instr->regs[0];
- return reg_num(dst) == REG_P0;
+ return (reg_num(dst) == REG_P0) && !(dst->flags &
IR3_REG_ARRAY);
}
return false;
}
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] freedreno: ir3: fix wrong return if reg is an array
On 10/24/18 11:05 PM, Rob Clark wrote:
On Tue, Oct 23, 2018 at 9:57 PM Hyunjun Ko wrote:
Since ir3_register struct has union, it could return true even
if it's an array register accidentally when checking whether it
is address/predicate register.
Fixes: dEQP-GLES31.functional.ssbo.layout.random.arrays_of_arrays.6
---
src/gallium/drivers/freedreno/ir3/ir3.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/gallium/drivers/freedreno/ir3/ir3.h
b/src/gallium/drivers/freedreno/ir3/ir3.h
index 3055c10f1d..db94603558 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3.h
+++ b/src/gallium/drivers/freedreno/ir3/ir3.h
@@ -739,7 +739,7 @@ static inline bool writes_addr(struct ir3_instruction
*instr)
{
if (instr->regs_count > 0) {
struct ir3_register *dst = instr->regs[0];
- return reg_num(dst) == REG_A0;
+ return (reg_num(dst) == REG_A0) && !(dst->flags &
IR3_REG_ARRAY);
hmm, good catch.. although I wonder if writes_gpr() in ir3_ra.c has
the same issue. Or anywhere else? (Otoh, I guess checking for CONST
and IMMED in writes_gpr() is a bit silly)
From a quick look at the IR3_REG_* flags, I think IR3_REG_ARRAY is the
only problematic case.. but this is a bit more fragile than it should
be. Maybe we should just move 'int num' out of the union? Maybe that
was too much premature optimization?
BR,
-R
Didn't figure out writes_gpr() might have same issue.
I think taking num out of union should avoid other similar issues if we
don't think
that would increase memory footprint that much.
Working on this.
}
return false;
}
@@ -748,7 +748,7 @@ static inline bool writes_pred(struct ir3_instruction
*instr)
{
if (instr->regs_count > 0) {
struct ir3_register *dst = instr->regs[0];
- return reg_num(dst) == REG_P0;
+ return (reg_num(dst) == REG_P0) && !(dst->flags &
IR3_REG_ARRAY);
}
return false;
}
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] freedreno: ir3: fix wrong return if reg is an array
On Tue, Oct 23, 2018 at 9:57 PM Hyunjun Ko wrote:
>
> Since ir3_register struct has union, it could return true even
> if it's an array register accidentally when checking whether it
> is address/predicate register.
>
> Fixes: dEQP-GLES31.functional.ssbo.layout.random.arrays_of_arrays.6
> ---
> src/gallium/drivers/freedreno/ir3/ir3.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/freedreno/ir3/ir3.h
> b/src/gallium/drivers/freedreno/ir3/ir3.h
> index 3055c10f1d..db94603558 100644
> --- a/src/gallium/drivers/freedreno/ir3/ir3.h
> +++ b/src/gallium/drivers/freedreno/ir3/ir3.h
> @@ -739,7 +739,7 @@ static inline bool writes_addr(struct ir3_instruction
> *instr)
> {
> if (instr->regs_count > 0) {
> struct ir3_register *dst = instr->regs[0];
> - return reg_num(dst) == REG_A0;
> + return (reg_num(dst) == REG_A0) && !(dst->flags &
> IR3_REG_ARRAY);
hmm, good catch.. although I wonder if writes_gpr() in ir3_ra.c has
the same issue. Or anywhere else? (Otoh, I guess checking for CONST
and IMMED in writes_gpr() is a bit silly)
From a quick look at the IR3_REG_* flags, I think IR3_REG_ARRAY is the
only problematic case.. but this is a bit more fragile than it should
be. Maybe we should just move 'int num' out of the union? Maybe that
was too much premature optimization?
BR,
-R
> }
> return false;
> }
> @@ -748,7 +748,7 @@ static inline bool writes_pred(struct ir3_instruction
> *instr)
> {
> if (instr->regs_count > 0) {
> struct ir3_register *dst = instr->regs[0];
> - return reg_num(dst) == REG_P0;
> + return (reg_num(dst) == REG_P0) && !(dst->flags &
> IR3_REG_ARRAY);
> }
> return false;
> }
> --
> 2.17.1
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] freedreno: ir3: fix wrong return if reg is an array
Since ir3_register struct has union, it could return true even
if it's an array register accidentally when checking whether it
is address/predicate register.
Fixes: dEQP-GLES31.functional.ssbo.layout.random.arrays_of_arrays.6
---
src/gallium/drivers/freedreno/ir3/ir3.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/gallium/drivers/freedreno/ir3/ir3.h
b/src/gallium/drivers/freedreno/ir3/ir3.h
index 3055c10f1d..db94603558 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3.h
+++ b/src/gallium/drivers/freedreno/ir3/ir3.h
@@ -739,7 +739,7 @@ static inline bool writes_addr(struct ir3_instruction
*instr)
{
if (instr->regs_count > 0) {
struct ir3_register *dst = instr->regs[0];
- return reg_num(dst) == REG_A0;
+ return (reg_num(dst) == REG_A0) && !(dst->flags &
IR3_REG_ARRAY);
}
return false;
}
@@ -748,7 +748,7 @@ static inline bool writes_pred(struct ir3_instruction
*instr)
{
if (instr->regs_count > 0) {
struct ir3_register *dst = instr->regs[0];
- return reg_num(dst) == REG_P0;
+ return (reg_num(dst) == REG_P0) && !(dst->flags &
IR3_REG_ARRAY);
}
return false;
}
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
