From 1a4153b24cce3f25db6bbc3dc5dc4efcfa146ab7 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Fri, 30 Aug 2019 17:29:17 -0700 Subject: [PATCH] pan/midgard: Fix cppcheck issues Miscellaneous minor issues flagged by cppcheck. Signed-off-by: Alyssa Rosenzweig Reviewed-by: Tomeu Vizoso --- src/panfrost/midgard/midgard_compile.c | 19 +++++++++---------- src/panfrost/midgard/midgard_emit.c | 5 +++-- src/panfrost/midgard/midgard_print.c | 6 +++--- src/panfrost/midgard/midgard_ra_pipeline.c | 3 +-- src/panfrost/midgard/midgard_schedule.c | 16 +++++++++++----- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/panfrost/midgard/midgard_compile.c b/src/panfrost/midgard/midgard_compile.c index 79f33e8006e..585591d9356 100644 --- a/src/panfrost/midgard/midgard_compile.c +++ b/src/panfrost/midgard/midgard_compile.c @@ -1264,16 +1264,15 @@ emit_ssbo_access( ins.src[is_read ? 0 : 1] = addr; /* TODO: What is this? It looks superficially like a shift << 5, but - * arg_1 doesn't take a shift Should it be E0 or A0? */ - if (indirect_offset) - ins.load_store.arg_1 |= 0xE0; - - /* We also need to emit the indirect offset */ + * arg_1 doesn't take a shift Should it be E0 or A0? We also need the + * indirect offset. */ - if (indirect_offset) + if (indirect_offset) { + ins.load_store.arg_1 |= 0xE0; ins.src[is_read ? 1 : 2] = nir_src_index(ctx, indirect_offset); - else + } else { ins.load_store.arg_2 = 0x7E; + } /* TODO: Bounds check */ @@ -1558,8 +1557,8 @@ emit_intrinsic(compiler_context *ctx, nir_intrinsic_instr *instr) reg = nir_dest_index(ctx, &instr->dest); assert(ctx->is_blend); - midgard_instruction ins = m_ld_color_buffer_8(reg, 0); - emit_mir_instruction(ctx, ins); + midgard_instruction ld = m_ld_color_buffer_8(reg, 0); + emit_mir_instruction(ctx, ld); break; case nir_intrinsic_load_blend_const_color_rgba: { @@ -2828,7 +2827,7 @@ midgard_compile_shader_nir(struct midgard_screen *screen, nir_shader *nir, midga fprintf(stderr, "shader%d - %s shader: " "%u inst, %u bundles, %u quadwords, " "%u registers, %u threads, %u loops, " - "%d:%d spills:fills\n", + "%u:%u spills:fills\n", SHADER_DB_COUNT++, gl_shader_stage_name(ctx->stage), nr_ins, nr_bundles, ctx->quadword_count, diff --git a/src/panfrost/midgard/midgard_emit.c b/src/panfrost/midgard/midgard_emit.c index 0d904f7166e..4f6ac05ab65 100644 --- a/src/panfrost/midgard/midgard_emit.c +++ b/src/panfrost/midgard/midgard_emit.c @@ -50,7 +50,6 @@ vector_to_scalar_source(unsigned u, bool is_int, bool is_full, /* TODO: Integers */ unsigned component = (v.swizzle >> (2*masked_component)) & 3; - bool upper = false; /* TODO */ midgard_scalar_alu_src s = { 0 }; @@ -69,8 +68,10 @@ vector_to_scalar_source(unsigned u, bool is_int, bool is_full, if (s.full) s.component = component << 1; - else + else { + bool upper = false; /* TODO */ s.component = component + (upper << 2); + } if (is_int) { /* TODO */ diff --git a/src/panfrost/midgard/midgard_print.c b/src/panfrost/midgard/midgard_print.c index 50eb626f5e8..c79603c64d2 100644 --- a/src/panfrost/midgard/midgard_print.c +++ b/src/panfrost/midgard/midgard_print.c @@ -174,7 +174,7 @@ mir_print_instruction(midgard_instruction *ins) void mir_print_block(midgard_block *block) { - printf("block%d: {\n", block->source_id); + printf("block%u: {\n", block->source_id); if (block->is_scheduled) { mir_foreach_bundle_in_block(block, bundle) { @@ -194,14 +194,14 @@ mir_print_block(midgard_block *block) if (block->nr_successors) { printf(" -> "); for (unsigned i = 0; i < block->nr_successors; ++i) { - printf("block%d%s", block->successors[i]->source_id, + printf("block%u%s", block->successors[i]->source_id, (i + 1) != block->nr_successors ? ", " : ""); } } printf(" from { "); mir_foreach_predecessor(block, pred) - printf("block%d ", pred->source_id); + printf("block%u ", pred->source_id); printf("}"); printf("\n\n"); diff --git a/src/panfrost/midgard/midgard_ra_pipeline.c b/src/panfrost/midgard/midgard_ra_pipeline.c index afbcf5b64a0..1dea92d053a 100644 --- a/src/panfrost/midgard/midgard_ra_pipeline.c +++ b/src/panfrost/midgard/midgard_ra_pipeline.c @@ -46,7 +46,6 @@ mir_pipeline_ins( unsigned pipeline_count) { midgard_instruction *ins = bundle->instructions[i]; - unsigned dest = ins->dest; /* We could be pipelining a register, so we need to make sure that all * of the components read in this bundle are written in this bundle, @@ -99,7 +98,7 @@ mir_pipeline_ins( /* We're only live in this bundle -- pipeline! */ - mir_rewrite_index(ctx, dest, SSA_FIXED_REGISTER(24 + pipeline_count)); + mir_rewrite_index(ctx, node, SSA_FIXED_REGISTER(24 + pipeline_count)); return true; } diff --git a/src/panfrost/midgard/midgard_schedule.c b/src/panfrost/midgard/midgard_schedule.c index 8f86701e33f..68b7a0b2fe4 100644 --- a/src/panfrost/midgard/midgard_schedule.c +++ b/src/panfrost/midgard/midgard_schedule.c @@ -170,9 +170,11 @@ can_writeout_fragment(compiler_context *ctx, midgard_instruction **bundle, unsig /* Simultaneously we scan for the set of dependencies */ size_t sz = sizeof(BITSET_WORD) * BITSET_WORDS(node_count); - BITSET_WORD *dependencies = alloca(sz); + BITSET_WORD *dependencies = calloc(1, sz); memset(dependencies, 0, sz); + bool success = false; + for (unsigned i = 0; i < count; ++i) { midgard_instruction *ins = bundle[i]; @@ -204,12 +206,12 @@ can_writeout_fragment(compiler_context *ctx, midgard_instruction **bundle, unsig /* Requirement 2 */ if (ins->unit == UNIT_VLUT) - return false; + goto done; } /* Requirement 1 */ if ((r0_written_mask & 0xF) != 0xF) - return false; + goto done; /* Requirement 3 */ @@ -217,11 +219,15 @@ can_writeout_fragment(compiler_context *ctx, midgard_instruction **bundle, unsig unsigned dest = bundle[i]->dest; if (dest < node_count && BITSET_TEST(dependencies, dest)) - return false; + goto done; } /* Otherwise, we're good to go */ - return true; + success = true; + +done: + free(dependencies); + return success; } /* Helpers for scheudling */ -- 2.30.2