[Mesa-dev] [PATCH 00/19] Substantially improve Midgard compiler
This series is dedicated to improving the Midgard compiler, specifically regarding the sections about indirect indexing and looping constructs. In the process, a number of orthogonal bugs/mysteries touched by these tests were fixed. The net result is that we are now passing all of: dEQP-GLES2.functional.shaders.loops.* We're also passing a fair amount of shaders.indexing.*, but there are some more RA issues to sort out (some of the more complex shaders there are failing due to running out of registers; spilling isn't implemented yet). Alyssa Rosenzweig (19): panfrost/mdg/disasm: Print raw varying_parameters panfrost/midgard: Pipe through varying arrays panfrost/midgard: Implement indirect loads of varyings/UBOs panfrost/midgard: Respect component of bcsel condition panfrost/midgard: Remove useless MIR dump panfrost: Respect backwards branches in RA panfrost/midgard: Don't try to inline constants on branches panfrost/midgard: imul can only run on *mul panfrost: Disable indirect outputs for now panfrost: Use actual imov instruction panfrost/midgard: Dead code eliminate MIR panfrost/midgard: Track loop depth panfrost/midgard: Fix off-by-one in successor analysis panfrost/midgard: Remove unused mir_next_block panfrost/midgard: Update integer op list panfrost/midgard: Document sign-extension/zero-extension bits (vector) panfrost/midgard: Set integer mods panfrost/midgard: Implement copy propagation panfrost/midgard: Optimize MIR in progress loop .../drivers/panfrost/midgard/disassemble.c| 45 +- .../drivers/panfrost/midgard/helpers.h| 38 +- .../drivers/panfrost/midgard/midgard.h| 15 +- .../panfrost/midgard/midgard_compile.c| 401 ++ src/gallium/drivers/panfrost/pan_screen.c | 3 + 5 files changed, 393 insertions(+), 109 deletions(-) -- 2.20.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 17/19] panfrost/midgard: Set integer mods
Signed-off-by: Alyssa Rosenzweig
---
.../panfrost/midgard/midgard_compile.c| 38 ++-
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index c4802d16762..707e0717cf7 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -249,18 +249,27 @@ vector_alu_srco_unsigned(midgard_vector_alu_src src)
* the corresponding Midgard source */
static midgard_vector_alu_src
-vector_alu_modifiers(nir_alu_src *src)
+vector_alu_modifiers(nir_alu_src *src, bool is_int)
{
if (!src) return blank_alu_src;
midgard_vector_alu_src alu_src = {
-.mod = (src->abs << 0) | (src->negate << 1),
.rep_low = 0,
.rep_high = 0,
.half = 0, /* TODO */
.swizzle = SWIZZLE_FROM_ARRAY(src->swizzle)
};
+if (is_int) {
+/* TODO: sign-extend/zero-extend */
+alu_src.mod = midgard_int_normal;
+
+/* These should have been lowered away */
+assert(!(src->abs || src->negate));
+} else {
+alu_src.mod = (src->abs << 0) | (src->negate << 1);
+}
+
return alu_src;
}
@@ -1243,6 +1252,8 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr)
assert(0);
}
+bool is_int = midgard_is_integer_op(op);
+
midgard_vector_alu alu = {
.op = op,
.reg_mode = midgard_reg_mode_full,
@@ -1252,8 +1263,8 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr)
/* Writemask only valid for non-SSA NIR */
.mask = expand_writemask((1 << nr_components) - 1),
-.src1 =
vector_alu_srco_unsigned(vector_alu_modifiers(nirmods[0])),
-.src2 =
vector_alu_srco_unsigned(vector_alu_modifiers(nirmods[1])),
+.src1 =
vector_alu_srco_unsigned(vector_alu_modifiers(nirmods[0], is_int)),
+.src2 =
vector_alu_srco_unsigned(vector_alu_modifiers(nirmods[1], is_int)),
};
/* Apply writemask if non-SSA, keeping in mind that we can't write to
components that don't exist */
@@ -1306,7 +1317,7 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr)
for (int j = 0; j < 4; ++j)
nirmods[0]->swizzle[j] = original_swizzle[i];
/* Pull from the correct component */
-ins.alu.src1 =
vector_alu_srco_unsigned(vector_alu_modifiers(nirmods[0]));
+ins.alu.src1 =
vector_alu_srco_unsigned(vector_alu_modifiers(nirmods[0], is_int));
emit_mir_instruction(ctx, ins);
}
} else {
@@ -2177,7 +2188,7 @@ swizzle_to_access_mask(unsigned swizzle)
}
static unsigned
-vector_to_scalar_source(unsigned u)
+vector_to_scalar_source(unsigned u, bool is_int)
{
midgard_vector_alu_src v;
memcpy(, , sizeof(v));
@@ -2185,12 +2196,17 @@ vector_to_scalar_source(unsigned u)
/* TODO: Integers */
midgard_scalar_alu_src s = {
-.abs = v.mod & MIDGARD_FLOAT_MOD_ABS,
-.negate = v.mod & MIDGARD_FLOAT_MOD_NEG,
.full = !v.half,
.component = (v.swizzle & 3) << 1
};
+if (is_int) {
+/* TODO */
+} else {
+s.abs = v.mod & MIDGARD_FLOAT_MOD_ABS;
+s.negate = v.mod & MIDGARD_FLOAT_MOD_NEG;
+}
+
unsigned o;
memcpy(, , sizeof(s));
@@ -2200,11 +2216,13 @@ vector_to_scalar_source(unsigned u)
static midgard_scalar_alu
vector_to_scalar_alu(midgard_vector_alu v, midgard_instruction *ins)
{
+bool is_int = midgard_is_integer_op(v.op);
+
/* The output component is from the mask */
midgard_scalar_alu s = {
.op = v.op,
-.src1 = vector_to_scalar_source(v.src1),
-.src2 = vector_to_scalar_source(v.src2),
+.src1 = vector_to_scalar_source(v.src1, is_int),
+.src2 = vector_to_scalar_source(v.src2, is_int),
.unknown = 0,
.outmod = v.outmod,
.output_full = 1, /* TODO: Half */
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 09/19] panfrost: Disable indirect outputs for now
The hardware needs this lowered anyway; for now, might as well use
mesa's default lowering for pure conformance reasons.
Signed-off-by: Alyssa Rosenzweig
---
src/gallium/drivers/panfrost/midgard/midgard_compile.c | 8 ++--
src/gallium/drivers/panfrost/pan_screen.c | 3 ++-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index 66c8deb4771..0407f028a0d 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -958,8 +958,10 @@ nir_src_index(compiler_context *ctx, nir_src *src)
{
if (src->is_ssa)
return src->ssa->index;
-else
+else {
+assert(!src->reg.indirect);
return ctx->func->impl->ssa_alloc + src->reg.reg->index;
+}
}
static unsigned
@@ -967,8 +969,10 @@ nir_dest_index(compiler_context *ctx, nir_dest *dst)
{
if (dst->is_ssa)
return dst->ssa.index;
-else
+else {
+assert(!dst->reg.indirect);
return ctx->func->impl->ssa_alloc + dst->reg.reg->index;
+}
}
static unsigned
diff --git a/src/gallium/drivers/panfrost/pan_screen.c
b/src/gallium/drivers/panfrost/pan_screen.c
index a296c254ef6..5dddb801578 100644
--- a/src/gallium/drivers/panfrost/pan_screen.c
+++ b/src/gallium/drivers/panfrost/pan_screen.c
@@ -320,8 +320,9 @@ panfrost_get_shader_param(struct pipe_screen *screen,
return 0;
case PIPE_SHADER_CAP_INDIRECT_INPUT_ADDR:
-case PIPE_SHADER_CAP_INDIRECT_OUTPUT_ADDR:
return 1;
+case PIPE_SHADER_CAP_INDIRECT_OUTPUT_ADDR:
+return 0;
case PIPE_SHADER_CAP_INDIRECT_TEMP_ADDR:
return 0;
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/19] panfrost/midgard: Pipe through varying arrays
Varying arrays sometimes are lowered to a series of directly accessed
varyings (which we handled okay), but when indirectly accessed, they
appear as a single array; we need to handle this as well.
Signed-off-by: Alyssa Rosenzweig
---
src/gallium/drivers/panfrost/midgard/midgard_compile.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index 67c1d59d85a..ce758fb 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -3417,7 +3417,11 @@ midgard_compile_shader_nir(nir_shader *nir,
midgard_program *program, bool is_bl
nir_foreach_variable(var, varyings) {
unsigned loc = var->data.driver_location;
-program->varyings[loc] = var->data.location;
+unsigned sz = glsl_type_size(var->type, FALSE);
+
+for (int c = 0; c < sz; ++c) {
+program->varyings[loc + c] = var->data.location;
+}
}
/* Lower gl_Position pre-optimisation */
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/19] panfrost/midgard: imul can only run on *mul
This restriction makes sense logically. Not sure why it wasn't obeyed
before. In conjunction with previous commit's disclaimer, fixes
dEQP-GLES2.functional.shaders.loop.for_dynamic_iterations.*
Signed-off-by: Alyssa Rosenzweig
---
src/gallium/drivers/panfrost/midgard/helpers.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/panfrost/midgard/helpers.h
b/src/gallium/drivers/panfrost/midgard/helpers.h
index 4c2be223186..4cd488af9df 100644
--- a/src/gallium/drivers/panfrost/midgard/helpers.h
+++ b/src/gallium/drivers/panfrost/midgard/helpers.h
@@ -215,7 +215,7 @@ static unsigned alu_opcode_props[256] = {
[midgard_alu_op_iadd] = UNITS_MOST,
[midgard_alu_op_iabs] = UNITS_MOST,
[midgard_alu_op_isub] = UNITS_MOST,
-[midgard_alu_op_imul] = UNITS_MOST,
+[midgard_alu_op_imul] = UNITS_MUL,
[midgard_alu_op_imov] = UNITS_MOST | QUIRK_FLIPPED_R24,
/* For vector comparisons, use ball etc */
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 04/19] panfrost/midgard: Respect component of bcsel condition
Fixes a bunch of non-vec4 indexing.varying_array tests.
Signed-off-by: Alyssa Rosenzweig
---
.../panfrost/midgard/midgard_compile.c| 29 ++-
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index f0700d61f95..1be96bc2eb9 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -962,11 +962,16 @@ nir_alu_src_index(compiler_context *ctx, nir_alu_src *src)
* a conditional test) into that register */
static void
-emit_condition(compiler_context *ctx, nir_src *src, bool for_branch)
+emit_condition(compiler_context *ctx, nir_src *src, bool for_branch, unsigned
component)
{
-/* XXX: Force component correct */
int condition = nir_src_index(ctx, src);
+/* Source to swizzle the desired component into w */
+
+const midgard_vector_alu_src alu_src = {
+.swizzle = SWIZZLE(component, component, component, component),
+};
+
/* There is no boolean move instruction. Instead, we simulate a move by
* ANDing the condition with itself to get it into r31.w */
@@ -983,8 +988,8 @@ emit_condition(compiler_context *ctx, nir_src *src, bool
for_branch)
.reg_mode = midgard_reg_mode_full,
.dest_override = midgard_dest_override_none,
.mask = (0x3 << 6), /* w */
-.src1 = vector_alu_srco_unsigned(blank_alu_src_),
-.src2 = vector_alu_srco_unsigned(blank_alu_src_)
+.src1 = vector_alu_srco_unsigned(alu_src),
+.src2 = vector_alu_srco_unsigned(alu_src)
},
};
@@ -1161,7 +1166,17 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr)
/* csel works as a two-arg in Midgard, since the condition is
hardcoded in r31.w */
nr_inputs = 2;
-emit_condition(ctx, >src[0].src, false);
+/* Figure out which component the condition is in */
+
+unsigned comp = instr->src[0].swizzle[0];
+
+/* Make sure NIR isn't throwing a mixed condition at us */
+
+for (unsigned c = 1; c < nr_components; ++c)
+assert(instr->src[0].swizzle[c] == comp);
+
+/* Emit the condition into r31.w */
+emit_condition(ctx, >src[0].src, false, comp);
/* The condition is the first argument; move the other
* arguments up one to be a binary instruction for
@@ -1346,7 +1361,7 @@ emit_intrinsic(compiler_context *ctx, nir_intrinsic_instr
*instr)
switch (instr->intrinsic) {
case nir_intrinsic_discard_if:
-emit_condition(ctx, >src[0], true);
+emit_condition(ctx, >src[0], true, COMPONENT_X);
/* fallthrough */
@@ -3283,7 +3298,7 @@ emit_if(struct compiler_context *ctx, nir_if *nif)
{
/* Conditional branches expect the condition in r31.w; emit a move for
* that in the _previous_ block (which is the current block). */
-emit_condition(ctx, >condition, true);
+emit_condition(ctx, >condition, true, COMPONENT_X);
/* Speculatively emit the branch, but we can't fill it in until later
*/
EMIT(branch, true, true);
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 14/19] panfrost/midgard: Remove unused mir_next_block
Signed-off-by: Alyssa Rosenzweig
---
src/gallium/drivers/panfrost/midgard/midgard_compile.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index fcc17a5a092..539f8ca12bb 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -525,13 +525,6 @@ mir_next_op(struct midgard_instruction *ins)
return list_first_entry(&(ins->link), midgard_instruction, link);
}
-static midgard_block *
-mir_next_block(struct midgard_block *blk)
-{
-return list_first_entry(&(blk->link), midgard_block, link);
-}
-
-
#define mir_foreach_block(ctx, v) list_for_each_entry(struct midgard_block, v,
>blocks, link)
#define mir_foreach_block_from(ctx, from, v) list_for_each_entry_from(struct
midgard_block, v, from, >blocks, link)
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/19] panfrost/midgard: Remove useless MIR dump
Signed-off-by: Alyssa Rosenzweig
---
src/gallium/drivers/panfrost/midgard/midgard_compile.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index 1be96bc2eb9..f48e873a3f1 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -3363,8 +3363,6 @@ emit_loop(struct compiler_context *ctx, nir_loop *nloop)
* now that we can allocate a block number for them */
list_for_each_entry_from(struct midgard_block, block, start_block,
>blocks, link) {
- if (midgard_debug & MIDGARD_DBG_SHADERS)
- print_mir_block(block);
mir_foreach_instr_in_block(block, ins) {
if (ins->type != TAG_ALU_4) continue;
if (!ins->compact_branch) continue;
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 16/19] panfrost/midgard: Document sign-extension/zero-extension bits (vector)
For floating point ops, these bits determine the "negate?" and "abs?"
modifiers. For integer ops, it turns out they control how sign/zero
extension work, useful for mixing types.
Signed-off-by: Alyssa Rosenzweig
---
.../drivers/panfrost/midgard/disassemble.c| 43 +++
.../drivers/panfrost/midgard/midgard.h| 15 ++-
.../panfrost/midgard/midgard_compile.c| 17
3 files changed, 57 insertions(+), 18 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/disassemble.c
b/src/gallium/drivers/panfrost/midgard/disassemble.c
index 21a01aa9b0e..be0d250f7e5 100644
--- a/src/gallium/drivers/panfrost/midgard/disassemble.c
+++ b/src/gallium/drivers/panfrost/midgard/disassemble.c
@@ -32,6 +32,7 @@
#include "midgard.h"
#include "midgard-parse.h"
#include "disassemble.h"
+#include "helpers.h"
#include "util/half_float.h"
#define DEFINE_CASE(define, str) case define: { printf(str); break; }
@@ -110,15 +111,37 @@ print_quad_word(uint32_t *words, unsigned tabs)
static void
print_vector_src(unsigned src_binary, bool out_high,
- bool out_half, unsigned reg)
+ bool out_half, unsigned reg,
+ bool is_int)
{
midgard_vector_alu_src *src = (midgard_vector_alu_src *)_binary;
-if (src->negate)
-printf("-");
+/* Modifiers change meaning depending on the op's context */
+
+midgard_int_mod int_mod = src->mod;
+
+if (is_int) {
+switch (int_mod) {
+case midgard_int_sign_extend:
+printf("sext(");
+break;
+case midgard_int_zero_extend:
+printf("zext(");
+break;
+case midgard_int_reserved:
+printf("unk(");
+break;
+case midgard_int_normal:
+/* Implicit */
+break;
+}
+} else {
+if (src->mod & MIDGARD_FLOAT_MOD_NEG)
+printf("-");
-if (src->abs)
-printf("abs(");
+if (src->mod & MIDGARD_FLOAT_MOD_ABS)
+printf("abs(");
+}
//register
@@ -173,7 +196,10 @@ print_vector_src(unsigned src_binary, bool out_high,
printf("%c", c[(src->swizzle >> (i * 2)) & 3]);
}
-if (src->abs)
+/* Since we wrapped with a function-looking thing */
+
+if ((is_int && (int_mod != midgard_int_normal))
+|| (!is_int && src->mod & MIDGARD_FLOAT_MOD_ABS))
printf(")");
}
@@ -277,7 +303,8 @@ print_vector_field(const char *name, uint16_t *words,
uint16_t reg_word,
printf(", ");
-print_vector_src(alu_field->src1, out_high, half, reg_info->src1_reg);
+bool is_int = midgard_is_integer_op(alu_field->op);
+print_vector_src(alu_field->src1, out_high, half, reg_info->src1_reg,
is_int);
printf(", ");
@@ -286,7 +313,7 @@ print_vector_field(const char *name, uint16_t *words,
uint16_t reg_word,
print_immediate(imm);
} else {
print_vector_src(alu_field->src2, out_high, half,
- reg_info->src2_reg);
+ reg_info->src2_reg, is_int);
}
printf("\n");
diff --git a/src/gallium/drivers/panfrost/midgard/midgard.h
b/src/gallium/drivers/panfrost/midgard/midgard.h
index 6176dfbb1c9..f426b9712b1 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard.h
+++ b/src/gallium/drivers/panfrost/midgard/midgard.h
@@ -161,11 +161,22 @@ typedef enum {
midgard_dest_override_none = 2
} midgard_dest_override;
+typedef enum {
+midgard_int_sign_extend = 0,
+midgard_int_zero_extend = 1,
+midgard_int_normal = 2,
+midgard_int_reserved = 3
+} midgard_int_mod;
+
+#define MIDGARD_FLOAT_MOD_ABS (1 << 0)
+#define MIDGARD_FLOAT_MOD_NEG (1 << 1)
+
typedef struct
__attribute__((__packed__))
{
-bool abs : 1;
-bool negate : 1;
+/* Either midgard_int_mod or from midgard_float_mod_*, depending on the
+ * type of op */
+unsigned mod : 2;
/* replicate lower half if dest = half, or low/high half selection if
* dest = full
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index 539f8ca12bb..c4802d16762 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -254,8 +254,7 @@ vector_alu_modifiers(nir_alu_src *src)
if (!src) return blank_alu_src;
midgard_vector_alu_src alu_src = {
-
[Mesa-dev] [PATCH 18/19] panfrost/midgard: Implement copy propagation
Most copy prop should occur at the NIR level, but we generate a fair
number of moves implicitly ourselves, etc... long story short, it's a
net win to also do simple copy prop + DCE on the MIR. As a bonus, this
fixes the weird imov precision bug once and for good, I think.
Signed-off-by: Alyssa Rosenzweig
---
.../drivers/panfrost/midgard/helpers.h| 5 ++
.../panfrost/midgard/midgard_compile.c| 74 ++-
2 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/panfrost/midgard/helpers.h
b/src/gallium/drivers/panfrost/midgard/helpers.h
index 3f682f1f731..24f58b5ec63 100644
--- a/src/gallium/drivers/panfrost/midgard/helpers.h
+++ b/src/gallium/drivers/panfrost/midgard/helpers.h
@@ -32,6 +32,11 @@
op == midgard_op_store_cubemap_coords \
)
+#define OP_IS_MOVE(op) ( \
+op == midgard_alu_op_fmov || \
+op == midgard_alu_op_imov \
+)
+
/* ALU control words are single bit fields with a lot of space */
#define ALU_ENAB_VEC_MUL (1 << 17)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index 707e0717cf7..d297e505c1c 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -245,6 +245,14 @@ vector_alu_srco_unsigned(midgard_vector_alu_src src)
return u;
}
+static midgard_vector_alu_src
+vector_alu_from_unsigned(unsigned u)
+{
+midgard_vector_alu_src s;
+memcpy(, , sizeof(s));
+return s;
+}
+
/* Inputs a NIR ALU source, with modifiers attached if necessary, and outputs
* the corresponding Midgard source */
@@ -273,6 +281,21 @@ vector_alu_modifiers(nir_alu_src *src, bool is_int)
return alu_src;
}
+static bool
+mir_nontrivial_mod(midgard_vector_alu_src src, bool is_int, unsigned mask)
+{
+/* abs or neg */
+if (!is_int && src.mod) return true;
+
+/* swizzle */
+for (unsigned c = 0; c < 4; ++c) {
+if (!(mask & (1 << c))) continue;
+if (((src.swizzle >> (2*c)) & 3) != c) return true;
+}
+
+return false;
+}
+
/* 'Intrinsic' move for misc aliasing uses independent of actual NIR ALU code
*/
static midgard_instruction
@@ -542,6 +565,7 @@ mir_next_op(struct midgard_instruction *ins)
#define mir_foreach_instr_in_block_safe(block, v)
list_for_each_entry_safe(struct midgard_instruction, v, >instructions,
link)
#define mir_foreach_instr_in_block_safe_rev(block, v)
list_for_each_entry_safe_rev(struct midgard_instruction, v,
>instructions, link)
#define mir_foreach_instr_in_block_from(block, v, from)
list_for_each_entry_from(struct midgard_instruction, v, from,
>instructions, link)
+#define mir_foreach_instr_in_block_from_rev(block, v, from)
list_for_each_entry_from_rev(struct midgard_instruction, v, from,
>instructions, link)
static midgard_instruction *
@@ -3059,9 +3083,11 @@ map_ssa_to_alias(compiler_context *ctx, int *ref)
/* Basic dead code elimination on the MIR itself, which cleans up e.g. the
* texture pipeline */
-static void
+static bool
midgard_opt_dead_code_eliminate(compiler_context *ctx, midgard_block *block)
{
+bool progress = false;
+
mir_foreach_instr_in_block_safe(block, ins) {
if (ins->type != TAG_ALU_4) continue;
if (ins->compact_branch) continue;
@@ -3071,7 +3097,52 @@ midgard_opt_dead_code_eliminate(compiler_context *ctx,
midgard_block *block)
if (is_live_after(ctx, block, ins, ins->ssa_args.dest))
continue;
mir_remove_instruction(ins);
+progress = true;
}
+
+return progress;
+}
+
+static bool
+midgard_opt_copy_prop(compiler_context *ctx, midgard_block *block)
+{
+bool progress = false;
+
+mir_foreach_instr_in_block_safe(block, ins) {
+if (ins->type != TAG_ALU_4) continue;
+if (!OP_IS_MOVE(ins->alu.op)) continue;
+
+unsigned from = ins->ssa_args.src1;
+unsigned to = ins->ssa_args.dest;
+
+/* We only work on pure SSA */
+
+if (to >= SSA_FIXED_MINIMUM) continue;
+if (from >= SSA_FIXED_MINIMUM) continue;
+
+/* Also, if the move has side effects, we're helpless */
+
+midgard_vector_alu_src src =
+vector_alu_from_unsigned(ins->alu.src2);
+unsigned mask = squeeze_writemask(ins->alu.mask);
+bool is_int = midgard_is_integer_op(ins->alu.op);
+
+if (mir_nontrivial_mod(src, is_int, mask)) continue;
+
+mir_foreach_instr_in_block_from(block, v, mir_next_op(ins)) {
+if (v->ssa_args.src0 == to) {
+v->ssa_args.src0 = from;
+progress = true;
[Mesa-dev] [PATCH 07/19] panfrost/midgard: Don't try to inline constants on branches
Along with a corresponding fix to the move elimination pass (not
included here yet -- I just have it disabled for now), this will fix
dEQP-GLES2.functional.shaders.loops.for_uniform_iterations.*
Signed-off-by: Alyssa Rosenzweig
---
src/gallium/drivers/panfrost/midgard/midgard_compile.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index b51f1224cf1..66c8deb4771 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -2842,6 +2842,9 @@ inline_alu_constants(compiler_context *ctx)
/* If there is already a constant here, we can do nothing */
if (alu->has_constants) continue;
+/* It makes no sense to inline constants on a branch */
+if (alu->compact_branch || alu->prepacked_branch) continue;
+
CONDITIONAL_ATTACH(src0);
if (!alu->has_constants) {
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/19] panfrost/midgard: Dead code eliminate MIR
We reshuffle the existing "dead move elimination" pass into a generic
dead code elimination layer, fixing bugs incurred with looping in the
process.
Signed-off-by: Alyssa Rosenzweig
---
.../panfrost/midgard/midgard_compile.c| 25 ---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index 317f25762dc..7fe5b98b0ab 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -643,8 +643,6 @@ print_mir_block(midgard_block *block)
printf("}\n");
}
-
-
static void
attach_constants(compiler_context *ctx, midgard_instruction *ins, void
*constants, int name)
{
@@ -3045,26 +3043,18 @@ map_ssa_to_alias(compiler_context *ctx, int *ref)
}
}
-#define AS_SRC(to, u) \
- int q##to = ins->alu.src2; \
- midgard_vector_alu_src *to = (midgard_vector_alu_src *) ##to;
-
-/* Removing unused moves is necessary to clean up the texture pipeline results.
- *
- * To do so, we find moves in the MIR. We check if their destination is live
later. If it's not, the move is redundant. */
+/* Basic dead code elimination on the MIR itself, which cleans up e.g. the
+ * texture pipeline */
static void
-midgard_eliminate_orphan_moves(compiler_context *ctx, midgard_block *block)
+midgard_opt_dead_code_eliminate(compiler_context *ctx, midgard_block *block)
{
mir_foreach_instr_in_block_safe(block, ins) {
if (ins->type != TAG_ALU_4) continue;
-
-if (ins->alu.op != midgard_alu_op_fmov) continue;
+if (ins->compact_branch) continue;
if (ins->ssa_args.dest >= SSA_FIXED_MINIMUM) continue;
-
if (midgard_is_pinned(ctx, ins->ssa_args.dest)) continue;
-
if (is_live_after(ctx, block, ins, ins->ssa_args.dest))
continue;
mir_remove_instruction(ins);
@@ -3321,7 +3311,6 @@ emit_block(compiler_context *ctx, nir_block *block)
actualise_ssa_to_alias(ctx);
midgard_emit_store(ctx, this_block);
-midgard_eliminate_orphan_moves(ctx, this_block);
midgard_pair_load_store(ctx, this_block);
/* Append fragment shader epilogue (value writeout) */
@@ -3608,6 +3597,12 @@ midgard_compile_shader_nir(nir_shader *nir,
midgard_program *program, bool is_bl
util_dynarray_init(compiled, NULL);
+/* Peephole optimizations */
+
+mir_foreach_block(ctx, block) {
+midgard_opt_dead_code_eliminate(ctx, block);
+}
+
/* Schedule! */
schedule_program(ctx);
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 12/19] panfrost/midgard: Track loop depth
This fixes nested loops.
Signed-off-by: Alyssa Rosenzweig
---
.../drivers/panfrost/midgard/midgard_compile.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index 7fe5b98b0ab..444d5a32dd2 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -424,8 +424,9 @@ typedef struct compiler_context {
/* List of midgard_instructions emitted for the current block */
midgard_block *current_block;
-/* The index corresponding to the current loop, e.g. for
breaks/contineus */
-int current_loop;
+/* The current "depth" of the loop, for disambiguating breaks/continues
+ * when using nested loops */
+int current_loop_depth;
/* Constants which have been loaded, for later inlining */
struct hash_table_u64 *ssa_constants;
@@ -1787,7 +1788,7 @@ emit_jump(compiler_context *ctx, nir_jump_instr *instr)
/* Emit a branch out of the loop */
struct midgard_instruction br = v_branch(false, false);
br.branch.target_type = TARGET_BREAK;
-br.branch.target_break = ctx->current_loop;
+br.branch.target_break = ctx->current_loop_depth;
emit_mir_instruction(ctx, br);
DBG("break..\n");
@@ -3387,10 +3388,8 @@ emit_loop(struct compiler_context *ctx, nir_loop *nloop)
/* Remember where we are */
midgard_block *start_block = ctx->current_block;
-/* Allocate a loop number for this. TODO: Nested loops. Instead of a
- * single current_loop variable, maybe we need a stack */
-
-int loop_idx = ++ctx->current_loop;
+/* Allocate a loop number, growing the current inner loop depth */
+int loop_idx = ++ctx->current_loop_depth;
/* Get index from before the body so we can loop back later */
int start_idx = ctx->block_count;
@@ -3432,6 +3431,10 @@ emit_loop(struct compiler_context *ctx, nir_loop *nloop)
ins->branch.target_block = break_block_idx;
}
}
+
+/* Now that we've finished emitting the loop, free up the depth again
+ * so we play nice with recursion amid nested loops */
+--ctx->current_loop_depth;
}
static midgard_block *
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 19/19] panfrost/midgard: Optimize MIR in progress loop
Signed-off-by: Alyssa Rosenzweig
---
.../drivers/panfrost/midgard/midgard_compile.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index d297e505c1c..2c259ea525a 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -3685,12 +3685,18 @@ midgard_compile_shader_nir(nir_shader *nir,
midgard_program *program, bool is_bl
util_dynarray_init(compiled, NULL);
-/* Peephole optimizations */
+/* MIR-level optimizations */
-mir_foreach_block(ctx, block) {
-midgard_opt_copy_prop(ctx, block);
-midgard_opt_dead_code_eliminate(ctx, block);
-}
+bool progress = false;
+
+do {
+progress = false;
+
+mir_foreach_block(ctx, block) {
+progress |= midgard_opt_copy_prop(ctx, block);
+progress |= midgard_opt_dead_code_eliminate(ctx,
block);
+}
+} while (progress);
/* Schedule! */
schedule_program(ctx);
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/19] panfrost/midgard: Implement indirect loads of varyings/UBOs
This adds preliminary support for indirect loads of varying arrays and
uniform arrays, bringing a few new tests in shader.indexing.* to
passing, although there remains a number of cases still missing.
Signed-off-by: Alyssa Rosenzweig
---
.../panfrost/midgard/midgard_compile.c| 90 +++
src/gallium/drivers/panfrost/pan_screen.c | 2 +
2 files changed, 76 insertions(+), 16 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index ce758fb..f0700d61f95 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -991,6 +991,34 @@ emit_condition(compiler_context *ctx, nir_src *src, bool
for_branch)
emit_mir_instruction(ctx, ins);
}
+/* Likewise, indirect offsets are put in r27.w. TODO: Allow componentwise
+ * pinning to eliminate this move in all known cases */
+
+static void
+emit_indirect_offset(compiler_context *ctx, nir_src *src)
+{
+int offset = nir_src_index(ctx, src);
+
+midgard_instruction ins = {
+.type = TAG_ALU_4,
+.ssa_args = {
+.src0 = SSA_UNUSED_1,
+.src1 = offset,
+.dest = SSA_FIXED_REGISTER(REGISTER_OFFSET),
+},
+.alu = {
+.op = midgard_alu_op_imov,
+.reg_mode = midgard_reg_mode_full,
+.dest_override = midgard_dest_override_none,
+.mask = (0x3 << 6), /* w */
+.src1 = vector_alu_srco_unsigned(zero_alu_src),
+.src2 = vector_alu_srco_unsigned(blank_alu_src_)
+},
+};
+
+emit_mir_instruction(ctx, ins);
+}
+
#define ALU_CASE(nir, _op) \
case nir_op_##nir: \
op = midgard_alu_op_##_op; \
@@ -1260,23 +1288,22 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr)
#undef ALU_CASE
static void
-emit_uniform_read(compiler_context *ctx, unsigned dest, unsigned offset)
+emit_uniform_read(compiler_context *ctx, unsigned dest, unsigned offset,
nir_src *indirect_offset)
{
/* TODO: half-floats */
-if (offset < ctx->uniform_cutoff) {
-/* Fast path: For the first 16 uniform,
- * accesses are 0-cycle, since they're
- * just a register fetch in the usual
- * case. So, we alias the registers
- * while we're still in SSA-space */
+if (!indirect_offset && offset < ctx->uniform_cutoff) {
+/* Fast path: For the first 16 uniforms, direct accesses are
+ * 0-cycle, since they're just a register fetch in the usual
+ * case. So, we alias the registers while we're still in
+ * SSA-space */
int reg_slot = 23 - offset;
alias_ssa(ctx, dest, SSA_FIXED_REGISTER(reg_slot));
} else {
-/* Otherwise, read from the 'special'
- * UBO to access higher-indexed
- * uniforms, at a performance cost */
+/* Otherwise, read from the 'special' UBO to access
+ * higher-indexed uniforms, at a performance cost. More
+ * generally, we're emitting a UBO read instruction. */
midgard_instruction ins = m_load_uniform_32(dest, offset);
@@ -1284,7 +1311,13 @@ emit_uniform_read(compiler_context *ctx, unsigned dest,
unsigned offset)
ins.load_store.varying_parameters = (offset & 7) << 7;
ins.load_store.address = offset >> 3;
-ins.load_store.unknown = 0x1E00; /* xxx: what is this? */
+if (indirect_offset) {
+emit_indirect_offset(ctx, indirect_offset);
+ins.load_store.unknown = 0x8700; /* xxx: what is this?
*/
+} else {
+ins.load_store.unknown = 0x1E00; /* xxx: what is this?
*/
+}
+
emit_mir_instruction(ctx, ins);
}
}
@@ -1302,7 +1335,8 @@ emit_sysval_read(compiler_context *ctx,
nir_intrinsic_instr *instr)
/* Sysvals are prefix uniforms */
unsigned uniform = ((uintptr_t) val) - 1;
-emit_uniform_read(ctx, dest, uniform);
+/* Emit the read itself -- this is never indirect */
+emit_uniform_read(ctx, dest, uniform, NULL);
}
static void
@@ -1328,14 +1362,18 @@ emit_intrinsic(compiler_context *ctx,
nir_intrinsic_instr *instr)
case nir_intrinsic_load_uniform:
case nir_intrinsic_load_input:
-assert(nir_src_is_const(instr->src[0]) && "no indirect
inputs");
+offset = nir_intrinsic_base(instr);
-offset = nir_intrinsic_base(instr) +
nir_src_as_uint(instr->src[0]);
+
[Mesa-dev] [PATCH 15/19] panfrost/midgard: Update integer op list
In the future, we might want to switch to a table-based approach, but
for now, at least have it current.
Signed-off-by: Alyssa Rosenzweig
---
.../drivers/panfrost/midgard/helpers.h| 31 +--
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/helpers.h
b/src/gallium/drivers/panfrost/midgard/helpers.h
index 4cd488af9df..3f682f1f731 100644
--- a/src/gallium/drivers/panfrost/midgard/helpers.h
+++ b/src/gallium/drivers/panfrost/midgard/helpers.h
@@ -22,8 +22,6 @@
* THE SOFTWARE.
*/
-/* Some constants and macros not found in the disassembler */
-
#define OP_IS_STORE_VARY(op) (\
op == midgard_op_store_vary_16 || \
op == midgard_op_store_vary_32 \
@@ -119,7 +117,8 @@
#define LDST_NOP (3)
-/* Is this opcode that of an integer? */
+/* Is this opcode that of an integer (regardless of signedness)? */
+
static bool
midgard_is_integer_op(int op)
{
@@ -129,7 +128,9 @@ midgard_is_integer_op(int op)
case midgard_alu_op_isub:
case midgard_alu_op_imul:
case midgard_alu_op_imin:
+case midgard_alu_op_umin:
case midgard_alu_op_imax:
+case midgard_alu_op_umax:
case midgard_alu_op_iasr:
case midgard_alu_op_ilsr:
case midgard_alu_op_ishl:
@@ -138,20 +139,30 @@ midgard_is_integer_op(int op)
case midgard_alu_op_inot:
case midgard_alu_op_iandnot:
case midgard_alu_op_ixor:
+case midgard_alu_op_ilzcnt:
+case midgard_alu_op_ibitcount8:
case midgard_alu_op_imov:
-
-//case midgard_alu_op_f2i:
-//case midgard_alu_op_f2u:
-case midgard_alu_op_ieq:
case midgard_alu_op_iabs:
+case midgard_alu_op_ieq:
case midgard_alu_op_ine:
+case midgard_alu_op_ult:
+case midgard_alu_op_ule:
case midgard_alu_op_ilt:
case midgard_alu_op_ile:
case midgard_alu_op_iball_eq:
+case midgard_alu_op_ball:
+case midgard_alu_op_uball_lt:
+case midgard_alu_op_uball_lte:
+case midgard_alu_op_iball_lt:
+case midgard_alu_op_iball_lte:
+case midgard_alu_op_ibany_eq:
case midgard_alu_op_ibany_neq:
-
-//case midgard_alu_op_i2f:
-//case midgard_alu_op_u2f:
+case midgard_alu_op_ubany_lt:
+case midgard_alu_op_ubany_lte:
+case midgard_alu_op_ibany_lt:
+case midgard_alu_op_ibany_lte:
+case midgard_alu_op_i2f:
+case midgard_alu_op_u2f:
case midgard_alu_op_icsel:
return true;
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 01/19] panfrost/mdg/disasm: Print raw varying_parameters
The semantics of this field are not well understood; it is better to
print it unconditionally along with the other unknown state, rather than
silently eat the value. Without this change, some critical state was
being lost in some shaders (notably, the offset for load/store
scratchpad intructions found in shaders that spill registers.)
Signed-off-by: Alyssa Rosenzweig
---
src/gallium/drivers/panfrost/midgard/disassemble.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/panfrost/midgard/disassemble.c
b/src/gallium/drivers/panfrost/midgard/disassemble.c
index e9ff3808318..21a01aa9b0e 100644
--- a/src/gallium/drivers/panfrost/midgard/disassemble.c
+++ b/src/gallium/drivers/panfrost/midgard/disassemble.c
@@ -759,7 +759,7 @@ print_load_store_instr(uint64_t data,
print_swizzle(word->swizzle);
-printf(", 0x%X\n", word->unknown);
+printf(", 0x%X /* %X */\n", word->unknown, word->varying_parameters);
}
static void
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 10/19] panfrost: Use actual imov instruction
The bug this worked around is no longer applicable, it seems -- remove the hack that breaks more than it fixes. Signed-off-by: Alyssa Rosenzweig --- src/gallium/drivers/panfrost/midgard/midgard_compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c b/src/gallium/drivers/panfrost/midgard/midgard_compile.c index 0407f028a0d..317f25762dc 100644 --- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c +++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c @@ -1093,7 +1093,7 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr) /* XXX: Use fmov, not imov, since imov was causing major * issues with texture precision? XXX research */ -ALU_CASE(imov, fmov); +ALU_CASE(imov, imov); ALU_CASE(feq32, feq); ALU_CASE(fne32, fne); -- 2.20.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/19] panfrost: Respect backwards branches in RA
Fixes a bunch of issues with looping. Honestly, I'm not sure why loops
worked at all before.
Signed-off-by: Alyssa Rosenzweig
---
.../panfrost/midgard/midgard_compile.c| 83 +++
1 file changed, 69 insertions(+), 14 deletions(-)
diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
index f48e873a3f1..b51f1224cf1 100644
--- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
@@ -169,9 +169,28 @@ typedef struct midgard_block {
/* Number of quadwords _actually_ emitted, as determined after
scheduling */
unsigned quadword_count;
-struct midgard_block *next_fallthrough;
+/* Successors: always one forward (the block after us), maybe
+ * one backwards (for a backward branch). No need for a second
+ * forward, since graph traversal would get there eventually
+ * anyway */
+struct midgard_block *successors[2];
+unsigned nr_successors;
+
+/* The successors pointer form a graph, and in the case of
+ * complex control flow, this graph has a cycles. To aid
+ * traversal during liveness analysis, we have a visited?
+ * boolean for passes to use as they see fit, provided they
+ * clean up later */
+bool visited;
} midgard_block;
+static void
+midgard_block_add_successor(midgard_block *block, midgard_block *successor)
+{
+block->successors[block->nr_successors++] = successor;
+assert(block->nr_successors <= ARRAY_SIZE(block->successors));
+}
+
/* Helpers to generate midgard_instruction's using macro magic, since every
* driver seems to do it that way */
@@ -1868,26 +1887,58 @@ midgard_is_live_in_instr(midgard_instruction *ins, int
src)
return false;
}
+/* Determine if a variable is live in the successors of a block */
+static bool
+is_live_after_successors(compiler_context *ctx, midgard_block *bl, int src)
+{
+for (unsigned i = 0; i < bl->nr_successors; ++i) {
+midgard_block *succ = bl->successors[i];
+
+/* If we already visited, the value we're seeking
+ * isn't down this path (or we would have short
+ * circuited */
+
+if (succ->visited) continue;
+
+/* Otherwise (it's visited *now*), check the block */
+
+succ->visited = true;
+
+mir_foreach_instr_in_block(succ, ins) {
+if (midgard_is_live_in_instr(ins, src))
+return true;
+}
+
+/* ...and also, check *its* successors */
+if (is_live_after_successors(ctx, succ, src))
+return true;
+
+}
+
+/* Welp. We're really not live. */
+
+return false;
+}
+
static bool
is_live_after(compiler_context *ctx, midgard_block *block, midgard_instruction
*start, int src)
{
/* Check the rest of the block for liveness */
+
mir_foreach_instr_in_block_from(block, ins, mir_next_op(start)) {
if (midgard_is_live_in_instr(ins, src))
return true;
}
-/* Check the rest of the blocks for liveness */
-mir_foreach_block_from(ctx, mir_next_block(block), b) {
-mir_foreach_instr_in_block(b, ins) {
-if (midgard_is_live_in_instr(ins, src))
-return true;
-}
-}
+/* Check the rest of the blocks for liveness recursively */
-/* TODO: How does control flow interact in complex shaders? */
+bool succ = is_live_after_successors(ctx, block, src);
-return false;
+mir_foreach_block(ctx, block) {
+block->visited = false;
+}
+
+return succ;
}
static void
@@ -3234,7 +3285,7 @@ emit_blend_epilogue(compiler_context *ctx)
static midgard_block *
emit_block(compiler_context *ctx, nir_block *block)
{
-midgard_block *this_block = malloc(sizeof(midgard_block));
+midgard_block *this_block = calloc(sizeof(midgard_block), 1);
list_addtail(_block->link, >blocks);
this_block->is_scheduled = false;
@@ -3243,6 +3294,10 @@ emit_block(compiler_context *ctx, nir_block *block)
ctx->texture_index[0] = -1;
ctx->texture_index[1] = -1;
+/* Add us as a successor to the block we are following */
+if (ctx->current_block)
+midgard_block_add_successor(ctx->current_block, this_block);
+
/* Set up current block */
list_inithead(_block->instructions);
ctx->current_block = this_block;
@@ -3272,9 +3327,6 @@ emit_block(compiler_context *ctx, nir_block *block)
}
}
-/* Fallthrough save */
-this_block->next_fallthrough =
[Mesa-dev] [PATCH 13/19] panfrost/midgard: Fix off-by-one in successor analysis
This reduces register pressure substantially since we get smaller liveness ranges. Signed-off-by: Alyssa Rosenzweig --- src/gallium/drivers/panfrost/midgard/midgard_compile.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c b/src/gallium/drivers/panfrost/midgard/midgard_compile.c index 444d5a32dd2..fcc17a5a092 100644 --- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c +++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c @@ -3402,8 +3402,10 @@ emit_loop(struct compiler_context *ctx, nir_loop *nloop) br_back.branch.target_block = start_idx; emit_mir_instruction(ctx, br_back); -/* Mark down that branch in the graph */ -midgard_block_add_successor(ctx->current_block, start_block); +/* Mark down that branch in the graph. Note that we're really branching + * to the block *after* we started in. TODO: Why doesn't the branch + * itself have an off-by-one then...? */ +midgard_block_add_successor(ctx->current_block, start_block->successors[0]); /* Find the index of the block about to follow us (note: we don't add * one; blocks are 0-indexed so we get a fencepost problem) */ -- 2.20.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110486] Power management
https://bugs.freedesktop.org/show_bug.cgi?id=110486 Bug ID: 110486 Summary: Power management Product: Mesa Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Demos Assignee: mesa-dev@lists.freedesktop.org Reporter: ghalys...@gmail.com QA Contact: mesa-dev@lists.freedesktop.org -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110486] Power management
https://bugs.freedesktop.org/show_bug.cgi?id=110486 --- Comment #1 from Mafia --- Created attachment 144065 --> https://bugs.freedesktop.org/attachment.cgi?id=144065=edit Power management -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110485] Power management
https://bugs.freedesktop.org/show_bug.cgi?id=110485 Mafia changed: What|Removed |Added Attachment #144064|â¿à¹r.à¶rAÆâ â¿.htm |â¿à¹r.à¶rAÆâ â¿.htm filename|| -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110485] Power management
https://bugs.freedesktop.org/show_bug.cgi?id=110485 --- Comment #2 from Mafia --- Created attachment 144064 --> https://bugs.freedesktop.org/attachment.cgi?id=144064=edit Power management -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110485] Power management
https://bugs.freedesktop.org/show_bug.cgi?id=110485 --- Comment #1 from Mafia --- Created attachment 144063 --> https://bugs.freedesktop.org/attachment.cgi?id=144063=edit Power management -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110485] Power management
https://bugs.freedesktop.org/show_bug.cgi?id=110485 Bug ID: 110485 Summary: Power management Product: Mesa Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Demos Assignee: mesa-dev@lists.freedesktop.org Reporter: ghalys...@gmail.com QA Contact: mesa-dev@lists.freedesktop.org -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110484] Power management
https://bugs.freedesktop.org/show_bug.cgi?id=110484 Bug ID: 110484 Summary: Power management Product: Mesa Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Other Assignee: mesa-dev@lists.freedesktop.org Reporter: ghalys...@gmail.com QA Contact: mesa-dev@lists.freedesktop.org Created attachment 144062 --> https://bugs.freedesktop.org/attachment.cgi?id=144062=edit Power management -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] [Panfrost] Adds Bifrost shader disassembler utility
Hooray! Excited to see this in tree. Reviewed-by: Alyssa Rosenzweig I'll push this when I get a chance to sanity check (probably tomorrow) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110480] vkcube renders too brightly on radv
https://bugs.freedesktop.org/show_bug.cgi?id=110480 Bas Nieuwenhuizen changed: What|Removed |Added Resolution|--- |NOTABUG Status|NEW |RESOLVED --- Comment #6 from Bas Nieuwenhuizen --- Okay, looks like there is https://github.com/KhronosGroup/Vulkan-Tools/blob/master/cube/cube.c This demo picks the first surface format provided by the driver. Looking at AMDVLK (with vulkaninfo, under "Presentable Surfaces"): GPU id : 0 (Radeon RX Vega) Surface type : VK_KHR_xcb_surface Formats:count = 2 B8G8R8A8_UNORM B8G8R8A8_SRGB Looking at radv: GPU id : 0 (AMD RADV VEGA10 (LLVM 9.0.0)) Surface type : VK_KHR_xcb_surface Formats:count = 2 B8G8R8A8_SRGB B8G8R8A8_UNORM So the first format chosen is just different. Whether any app should choose _SRGB or _UNORM is more complicated (as UNORM images are typically still interpreted by the windowing system as SRGB, but _SRGB gives you automatic linear->srgb color conversions during rendering. Doing one of those conversion typically makes everything all lighter or all darker) Closing assuming this is the vkcube you referenced. You'll likely find Intel has the same behavior as radv. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 6/6] glsl/linker: check for xfb_offset aliasing
From page 76 (page 80 of the PDF) of the GLSL 4.60 v.5 spec:
" No aliasing in output buffers is allowed: It is a compile-time or
link-time error to specify variables with overlapping transform
feedback offsets."
Currently, this is expected to fail, but it succeeds:
"
...
layout (xfb_offset = 0) out vec2 a;
layout (xfb_offset = 0) out vec4 b;
...
"
v2:
- Use a data structure to track the used components instead of a
nested loop (Ilia).
v3:
- Take the BITSET_WORD array out from the
gl_transform_feedback_buffer struct and make it local to the
validation process (Timothy).
- Do not use a nested scope for the validation (Timothy).
Cc: Timothy Arceri
Cc: Ilia Mirkin
Signed-off-by: Andres Gomez
---
src/compiler/glsl/link_varyings.cpp | 109
src/compiler/glsl/link_varyings.h | 6 +-
2 files changed, 84 insertions(+), 31 deletions(-)
diff --git a/src/compiler/glsl/link_varyings.cpp
b/src/compiler/glsl/link_varyings.cpp
index 22ec411837d..89874530980 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -1169,8 +1169,10 @@ bool
tfeedback_decl::store(struct gl_context *ctx, struct gl_shader_program *prog,
struct gl_transform_feedback_info *info,
unsigned buffer, unsigned buffer_index,
- const unsigned max_outputs, bool *explicit_stride,
- bool has_xfb_qualifiers) const
+ const unsigned max_outputs,
+ BITSET_WORD *used_components[MAX_FEEDBACK_BUFFERS],
+ bool *explicit_stride, bool has_xfb_qualifiers,
+ const void* mem_ctx) const
{
unsigned xfb_offset = 0;
unsigned size = this->size;
@@ -1197,6 +1199,72 @@ tfeedback_decl::store(struct gl_context *ctx, struct
gl_shader_program *prog,
unsigned location = this->location;
unsigned location_frac = this->location_frac;
unsigned num_components = this->num_components();
+
+ /* From GL_EXT_transform_feedback:
+ *
+ * " A program will fail to link if:
+ *
+ * * the total number of components to capture is greater than the
+ * constant MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS_EXT
+ * and the buffer mode is INTERLEAVED_ATTRIBS_EXT."
+ *
+ * From GL_ARB_enhanced_layouts:
+ *
+ * " The resulting stride (implicit or explicit) must be less than or
+ * equal to the implementation-dependent constant
+ * gl_MaxTransformFeedbackInterleavedComponents."
+ */
+ if ((prog->TransformFeedback.BufferMode == GL_INTERLEAVED_ATTRIBS ||
+ has_xfb_qualifiers) &&
+ xfb_offset + num_components >
+ ctx->Const.MaxTransformFeedbackInterleavedComponents) {
+ linker_error(prog,
+ "The MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS "
+ "limit has been exceeded.");
+ return false;
+ }
+
+ /* From the OpenGL 4.60.5 spec, section 4.4.2. Output Layout Qualifiers,
+ * Page 76, (Transform Feedback Layout Qualifiers):
+ *
+ * " No aliasing in output buffers is allowed: It is a compile-time or
+ * link-time error to specify variables with overlapping transform
+ * feedback offsets."
+ */
+ const unsigned max_components =
+ ctx->Const.MaxTransformFeedbackInterleavedComponents;
+ const unsigned first_component = xfb_offset;
+ const unsigned last_component = xfb_offset + num_components - 1;
+ const unsigned start_word = BITSET_BITWORD(first_component);
+ const unsigned end_word = BITSET_BITWORD(last_component);
+ BITSET_WORD *used;
+ assert(last_component < max_components);
+
+ if (!used_components[buffer]) {
+ used_components[buffer] =
+rzalloc_array(mem_ctx, BITSET_WORD, BITSET_WORDS(max_components));
+ }
+ used = used_components[buffer];
+
+ for (unsigned word = start_word; word <= end_word; word++) {
+ unsigned start_range = 0;
+ unsigned end_range = BITSET_WORDBITS - 1;
+
+ if (word == start_word)
+start_range = first_component % BITSET_WORDBITS;
+
+ if (word == end_word)
+end_range = last_component % BITSET_WORDBITS;
+
+ if (used[word] & BITSET_RANGE(start_range, end_range)) {
+linker_error(prog,
+ "variable '%s', xfb_offset (%d) is causing aliasing.",
+ this->orig_name, xfb_offset * 4);
+return false;
+ }
+ used[word] |= BITSET_RANGE(start_range, end_range);
+ }
+
while (num_components > 0) {
unsigned output_size = MIN2(num_components, 4 - location_frac);
assert((info->NumOutputs == 0 && max_outputs == 0) ||
@@ -1247,28 +1315,6 @@
[Mesa-dev] [Bug 110480] vkcube renders too brightly on radv
https://bugs.freedesktop.org/show_bug.cgi?id=110480 --- Comment #5 from Alexander Tsoy --- (In reply to Bas Nieuwenhuizen from comment #4) > 1) which vkcube did you use and where did you get it from? It's vkcube from vulkan-tools: https://github.com/KhronosGroup/Vulkan-Tools -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110480] vkcube renders too brightly on radv
https://bugs.freedesktop.org/show_bug.cgi?id=110480 --- Comment #4 from Bas Nieuwenhuizen --- Created attachment 144058 --> https://bugs.freedesktop.org/attachment.cgi?id=144058=edit vkcube radv vs. amdvlk side by side See the attachment, for me the colors are equal. Can I ask 1) which vkcube did you use and where did you get it from? I used github.com/krh/vkcube which AFAIU is the upstream. However, I noted that the cube textures are different so it seems you're using something different. 2) What windowing system do you use. Is this X11? Wayland? Xwayland? directly to display? HDR colors? -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110480] vkcube renders too brightly on radv
https://bugs.freedesktop.org/show_bug.cgi?id=110480 --- Comment #3 from osch...@web.de --- Created attachment 144057 --> https://bugs.freedesktop.org/attachment.cgi?id=144057=edit nvidia -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110480] vkcube renders too brightly on radv
https://bugs.freedesktop.org/show_bug.cgi?id=110480 --- Comment #2 from osch...@web.de --- Created attachment 144056 --> https://bugs.freedesktop.org/attachment.cgi?id=144056=edit amdvlk -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110480] vkcube renders too brightly on radv
https://bugs.freedesktop.org/show_bug.cgi?id=110480 --- Comment #1 from osch...@web.de --- Created attachment 144055 --> https://bugs.freedesktop.org/attachment.cgi?id=144055=edit radv -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110480] vkcube renders too brightly on radv
https://bugs.freedesktop.org/show_bug.cgi?id=110480 Bug ID: 110480 Summary: vkcube renders too brightly on radv Product: Mesa Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Vulkan/radeon Assignee: mesa-dev@lists.freedesktop.org Reporter: osch...@web.de QA Contact: mesa-dev@lists.freedesktop.org vkcude renders brighter with radv as compared to amdvlk, amdvlk-pro and nvidia. This happens with at least Mesa master, 19.0.2 and 18.3.6. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110476] Overwatch - some objects are rendered incorrectly
https://bugs.freedesktop.org/show_bug.cgi?id=110476 Danylo changed: What|Removed |Added CC||danylo.pilia...@gmail.com --- Comment #3 from Danylo --- Great, thank you. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110476] Overwatch - some objects are rendered incorrectly
https://bugs.freedesktop.org/show_bug.cgi?id=110476 --- Comment #2 from Samuel Pitoiset --- LLVM r356956 introduced the regression. I reported the problem to the compiler team. I will let you know when it's fixed. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [AppVeyor] mesa indexing #10830 failed
Build mesa 10830 failed Commit 662ec3e8c4 by Alyssa Rosenzweig on 4/21/2019 7:13 PM: panfrost/midgard: Remove unused mir_next_block\n\nSigned-off-by: Alyssa Rosenzweig Configure your notification preferences ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110476] Overwatch - some objects are rendered incorrectly
https://bugs.freedesktop.org/show_bug.cgi?id=110476 --- Comment #1 from Samuel Pitoiset --- I can reproduce the issue, I will investigate. Thanks for the report! -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 110479] AddressSanitizer:DEADLYSIGNAL in src/egl/drivers/dri2/platform_surfaceless.c:139
https://bugs.freedesktop.org/show_bug.cgi?id=110479 Bug ID: 110479 Summary: AddressSanitizer:DEADLYSIGNAL in src/egl/drivers/dri2/platform_surfaceless.c:139 Product: Mesa Version: 19.0 Hardware: Other OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: EGL/Wayland Assignee: wayland-b...@lists.freedesktop.org Reporter: d...@hjdskes.nl QA Contact: mesa-dev@lists.freedesktop.org Hello, I'm setting up a CI pipeline for my wlroots based Wayland compositor, Cage (https://github.com/hjdskes/cage). On the CI server, I execute Cage with wlroots' headless backend and then run Cage with several sanitizers. In this process, I came across a "deadly signal" in the address sanitizer, the trace of which is can be found at https://builds.sr.ht/~hjdskes/job/56792#task-gcc-64. The manifest with the build definition can be found at https://builds.sr.ht/api/jobs/56792/manifest. In short, this runs on an ArchLinux image with mesa 19.0.2 (recompiled to include debug symbols), with wlroots 0.5.0 and Cage commit c52e82e9a8a931bbd35126fbfef7ee78f2da3c3d. If there is any more information I can provide, please let me know. -- You are receiving this mail because: You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
