[Mesa-dev] [PATCH v5 2/6] mesa/st: glsl_to_tgsi: implement new temporary register lifetime tracker
Nicolai Hähnle
nhaehnle at gmail.com
Mon Jun 26 12:52:48 UTC 2017
Thanks for the update. First off, you're still not tracking individual
components, but that's absolute necessary. Think:
BGNLOOP
MOV TEMP[1].x, ...
UIF ...
MOV TEMP[1].y, ...
ENDIF
use TEMP[1].y
ENDLOOP
Now for a scan through the patch:
On 25.06.2017 09:22, Gert Wollny wrote:
> This patch adds a class for tracking the life times of temporary registers
> in the glsl to tgsi translation. The algorithm runs in three steps:
> First, in order to minimize the number of needed memory allocations the
> program is scanned to evaluate the number of scopes.
> Then, the program is scanned second time to recorc the important register
Typo: record
> access time points: first and last reads and writes and their link to the
> execution scope (loop, if/else branch, switch case).
> In the third step for each register the actuall minimal life time is
Typo: actual (though you could just drop the word entirely)
> evaluated.
> ---
> src/mesa/Makefile.sources | 2 +
> .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 662 +++++++++++++++++++++
> .../state_tracker/st_glsl_to_tgsi_temprename.h | 33 +
> 3 files changed, 697 insertions(+)
> create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h
>
> diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
> index 21f9167bda..2359ec3c7d 100644
> --- a/src/mesa/Makefile.sources
> +++ b/src/mesa/Makefile.sources
> @@ -509,6 +509,8 @@ STATETRACKER_FILES = \
> state_tracker/st_glsl_to_tgsi.h \
> state_tracker/st_glsl_to_tgsi_private.cpp \
> state_tracker/st_glsl_to_tgsi_private.h \
> + state_tracker/st_glsl_to_tgsi_temprename.cpp \
> + state_tracker/st_glsl_to_tgsi_temprename.h \
Looks like inconsistent whitespace.
> state_tracker/st_glsl_types.cpp \
> state_tracker/st_glsl_types.h \
> state_tracker/st_manager.c \
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> new file mode 100644
> index 0000000000..729d77130e
> --- /dev/null
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
> @@ -0,0 +1,662 @@
> +/*
> + * Copyright © 2017 Gert Wollny
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +
> +#include "st_glsl_to_tgsi_temprename.h"
> +#include <tgsi/tgsi_info.h>
> +#include <mesa/program/prog_instruction.h>
> +#include <limits>
> +
> +/* Without c++11 define the nullptr for forward-compatibility
> + * and better readibility */
> +#if __cplusplus < 201103L
> +#define nullptr 0
> +#endif
> +
> +using std::numeric_limits;
> +
> +enum e_scope_type {
Please drop the "e_" prefix here and below, we don't usually do that.
> + sct_outer,
> + sct_loop,
> + sct_if,
> + sct_else,
> + sct_switch,
> + sct_switch_case,
> + sct_switch_default,
> + sct_unknown
> +};
> +
> +enum e_acc_type {
> + acc_read,
> + acc_write,
> + acc_write_cond_from_self
Document what this means.
> +};
> +
> +class prog_scope {
> +
> +public:
> + prog_scope(prog_scope *parent, e_scope_type type, int id, int depth,
> + int begin);
> +
> + e_scope_type type() const;
> + prog_scope *parent() const;
> + int nesting_depth() const;
> + int id() const;
> + int end() const;
> + int begin() const;
> + int loop_continue_line() const;
> +
> + const prog_scope *in_ifelse_scope() const;
> + const prog_scope *in_switchcase_scope() const;
> + const prog_scope *innermost_loop() const;
> + const prog_scope *outermost_loop() const;
> +
> + bool in_loop() const;
> + bool is_conditional() const;
> + bool break_is_for_switchcase() const;
> + bool contains(const prog_scope& other) const;
Please stick to pointers here. (The rule of thumb I use is that if you
care about the identity of a C++ object rather than its contents, you
should not use references.)
> +
> + void set_end(int end);
> + void set_previous_case_scope(prog_scope *prev);
> + void set_continue_line(int line);
> +
> +private:
> + e_scope_type scope_type;
> + int scope_id;
> + int scope_nesting_depth;
> + int scope_begin;
> + int scope_end;
> + int loop_cont_line;
> + prog_scope *previous_case_scope;
> + prog_scope *parent_scope;
> +};
> +
> +class temp_access {
> +public:
> + temp_access();
> + void record(int line, e_acc_type rw, prog_scope *scope);
> + lifetime get_required_lifetime();
> +private:
> + prog_scope *last_read_scope;
> + prog_scope *first_read_scope;
> + prog_scope *first_write_scope;
> + int first_dominant_write;
> + int last_read;
> + int last_write;
> + int first_read;
> + bool keep_for_full_loop;
> +};
> +
> +/* Some storage class to encapsulate the prog_scope (de-)allocations */
> +class prog_scope_storage {
> +public:
> + prog_scope_storage(void *mem_ctx, int n);
> + ~prog_scope_storage();
> + prog_scope * create(prog_scope *p, e_scope_type type, int id,
> + int lvl, int s_begin);
> +private:
> + void *mem_ctx;
> + int current_slot;
> + prog_scope *storage;
> +};
> +
> +/* Scan the program and estimate the required register life times.
> + * The array lifetimes must be pre-allocated */
Closing */ on its own line.
> +void
> +get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions,
> + int ntemps, struct lifetime *lifetimes)
> +{
> +
Remove whitespace line.
> + int line = 0;
> + int loop_id = 0;
> + int if_id = 0;
> + int switch_id = 0;
> + int scope_level = 0;
> + bool is_at_end = false;
> +
Remove whitespace line.
> + int n_scopes = 1;
> +
> +
> +
Collapse whitespace lines. We never have consecutive empty lines inside
function, struct, etc. declarations. The only place where two
consecutive empty lines are occasionally used is at the top level (file)
scope.
> + /* count scopes to allocate the needed space without the need for
> + * re-allocation */
> + foreach_in_list(glsl_to_tgsi_instruction, inst, instructions) {
> + if (inst->op == TGSI_OPCODE_BGNLOOP ||
> + inst->op == TGSI_OPCODE_SWITCH ||
> + inst->op == TGSI_OPCODE_CASE ||
> + inst->op == TGSI_OPCODE_IF ||
> + inst->op == TGSI_OPCODE_UIF ||
> + inst->op == TGSI_OPCODE_ELSE ||
> + inst->op == TGSI_OPCODE_DEFAULT)
> + ++n_scopes;
> + }
> +
> + prog_scope_storage scopes(mem_ctx, n_scopes);
> + temp_access *acc = new temp_access[ntemps];
> + prog_scope *cur_scope = scopes.create(nullptr, sct_outer, 0,
> + scope_level++, line);
> +
> + foreach_in_list(glsl_to_tgsi_instruction, inst, instructions) {
> + if (is_at_end) {
> + assert(!"GLSL_TO_TGSI: shader has instructions past end marker");
> + break;
> + }
> +
> + switch (inst->op) {
> + case TGSI_OPCODE_BGNLOOP: {
> + cur_scope = scopes.create(cur_scope, sct_loop, loop_id++,
> + scope_level++, line);
> + break;
> + }
> + case TGSI_OPCODE_ENDLOOP: {
> + --scope_level;
> + cur_scope->set_end(line);
> + cur_scope = cur_scope->parent();
> + assert(cur_scope);
> + break;
> + }
> + case TGSI_OPCODE_IF:
> + case TGSI_OPCODE_UIF:{
Space before {
> + for (unsigned j = 0; j < num_inst_src_regs(inst); j++) {
> + if (inst->src[j].file == PROGRAM_TEMPORARY)
> + acc[inst->src[j].index].record(line, acc_read, cur_scope);
> + }
> + cur_scope = scopes.create(cur_scope, sct_if, if_id++,
> + scope_level++, line+1);
> + break;
> + }
> + case TGSI_OPCODE_ELSE: {
> + cur_scope->set_end(line-1);
> + cur_scope = scopes.create(cur_scope->parent(), sct_else,
> + cur_scope->id(),cur_scope->nesting_depth(),
> + line+1);
> + break;
> + }
> + case TGSI_OPCODE_END:{
> + cur_scope->set_end(line);
> + is_at_end = true;
> + break;
> + }
> + case TGSI_OPCODE_ENDIF:{
> + --scope_level;
> + cur_scope->set_end(line-1);
> + cur_scope = cur_scope->parent();
> + assert(cur_scope);
> + break;
> + }
> + case TGSI_OPCODE_SWITCH: {
> + cur_scope = scopes.create(cur_scope, sct_switch, switch_id++,
> + scope_level++, line);
> + break;
> + }
> + case TGSI_OPCODE_ENDSWITCH: {
> + --scope_level;
> + cur_scope->set_end(line-1);
> + /* remove the case level, it might not have been
> + * closed with a break */
Closing */ on its own line.
> + if (cur_scope->type() != sct_switch )
> + cur_scope = cur_scope->parent();
> +
> + cur_scope = cur_scope->parent();
> + assert(cur_scope);
> + break;
> + }
> + case TGSI_OPCODE_CASE:
> + case TGSI_OPCODE_DEFAULT: {
> + /* Switch cases and default are handled at the same nesting level
> + * like their enclosing switch */
Why? It seems surprising to mess with the invariant that
parent->nesting_depth() == nesting_depth() - 1.
> + e_scope_type t = inst->op == TGSI_OPCODE_CASE ? sct_switch_case
> + : sct_switch_default;
> + prog_scope *switch_scope = cur_scope;
> + if ( cur_scope->type() == sct_switch ) {
> + cur_scope = scopes.create(cur_scope, t, cur_scope->id(),
> + scope_level, line+1);
> + } else {
> + switch_scope = cur_scope->parent();
> + assert(switch_scope->type() == sct_switch);
> + prog_scope *scope = scopes.create(switch_scope, t,
> + switch_scope->id(),
> + switch_scope->nesting_depth(),
> + line);
> +
> + /* Previous case falls through */
> + if (cur_scope->end() == -1)
> + scope->set_previous_case_scope(cur_scope);
> +
> + cur_scope = scope;
> + }
> + for (unsigned j = 0; j < num_inst_src_regs(inst); j++) {
> + if (inst->src[j].file == PROGRAM_TEMPORARY)
> + acc[inst->src[j].index].record(line, acc_read, switch_scope);
> + }
> + }
> + case TGSI_OPCODE_BRK: {
> + if (cur_scope->break_is_for_switchcase()) {
> + cur_scope->set_end(line-1);
> + break;
> + }
> + }
> + case TGSI_OPCODE_CONT: {
> + cur_scope->set_continue_line(line);
I'm still frankly confused about the way you choose to handle BRK/CONT
in loops, and suspect you're doing it wrong. At the very least, having a
function called "set_continue_line" be called for a BRK is bad naming.
> + break;
> + }
> + default: {
> + for (unsigned j = 0; j < num_inst_src_regs(inst); j++) {
> + if (inst->src[j].file == PROGRAM_TEMPORARY)
> + acc[inst->src[j].index].record(line, acc_read, cur_scope);
> + }
> + for (unsigned j = 0; j < inst->tex_offset_num_offset; j++) {
> + if (inst->tex_offsets[j].file == PROGRAM_TEMPORARY)
> + acc[inst->tex_offsets[j].index].record(line, acc_read, cur_scope);
> + }
> +
> + e_acc_type write_type = inst->op == TGSI_OPCODE_UCMP ?
Despite the opcode being called "Integer Conditional Move", it does
write to dst unconditionally. It should probably have been called
"select" or something like that.
> + acc_write_cond_from_self :
> + acc_write;
> + for (unsigned j = 0; j < num_inst_dst_regs(inst); j++) {
> + if (inst->dst[j].file == PROGRAM_TEMPORARY)
> + acc[inst->dst[j].index].record(line, write_type, cur_scope);
> + }
> + }
> + }
> + ++line;
> + }
> +
> + /* make sure last scope is closed, even though no
> + * TGSI_OPCODE_END was given */
> + if (cur_scope->end() < 0)
> + cur_scope->set_end(line-1);
> +
> + for(int i = 1; i < ntemps; ++i)
> + lifetimes[i] = acc[i].get_required_lifetime();
> +
> + delete[] acc;
> +}
> +
> +prog_scope::prog_scope(prog_scope *parent, e_scope_type type, int id,
> + int depth, int scope_begin):
> + scope_type(type),
> + scope_id(id),
> + scope_nesting_depth(depth),
> + scope_begin(scope_begin),
> + scope_end(-1),
> + loop_cont_line(numeric_limits<int>::max()),
> + parent_scope(parent)
> +{
> +}
> +
> +e_scope_type prog_scope::type() const
> +{
> + return scope_type;
> +}
> +
> +
Be consistent about how you use whitespace.
> +prog_scope *prog_scope::parent() const
> +{
> + return parent_scope;
> +}
> +
> +int prog_scope::nesting_depth() const
> +{
> + return scope_nesting_depth;
> +}
> +
> +bool prog_scope::in_loop() const
> +{
> + if (scope_type == sct_loop)
> + return true;
> +
> + if (parent_scope)
> + return parent_scope->in_loop();
> +
> + return false;
> +}
> +
> +const prog_scope *prog_scope::innermost_loop() const
> +{
> + if (scope_type == sct_loop)
> + return this;
> +
> + if (parent_scope)
> + return parent_scope->innermost_loop();
> +
> + return nullptr;
> +}
> +
> +const prog_scope *prog_scope::outermost_loop() const
> +{
> + const prog_scope *loop = nullptr;
> + const prog_scope *p = this;
> +
> + do {
> +
Remove empty line.
> + if (p->type() == sct_loop)
> + loop = p;
> +
> + p = p->parent();
> +
Remove empty line.
> + } while (p);
> +
> + return loop;
> +}
> +
> +bool prog_scope::contains(const prog_scope& other) const
> +{
> + return (begin() <= other.begin()) && (end() >= other.end());
> +}
> +
> +bool prog_scope::is_conditional() const
> +{
> + return scope_type == sct_if ||
> + scope_type == sct_else ||
> + scope_type == sct_switch_case ||
> + scope_type == sct_switch_default;
> +}
> +
> +const prog_scope *prog_scope::in_ifelse_scope() const
> +{
> + if (scope_type == sct_if ||
> + scope_type == sct_else)
> + return this;
> +
> + if (parent_scope)
> + return parent_scope->in_ifelse_scope();
> +
> + return nullptr;
> +}
> +
> +const prog_scope *prog_scope::in_switchcase_scope() const
> +{
> + if (scope_type == sct_switch_case ||
> + scope_type == sct_switch_default)
> + return this;
> +
> + if (parent_scope)
> + return parent_scope->in_switchcase_scope();
> +
> + return nullptr;
> +}
> +
> +bool prog_scope::break_is_for_switchcase() const
> +{
> + if (scope_type == sct_loop)
> + return false;
> +
> + if (scope_type == sct_switch_case ||
> + scope_type == sct_switch_default ||
> + scope_type == sct_switch)
> + return true;
> +
> + if (parent_scope)
> + return parent_scope->break_is_for_switchcase();
> +
> + return false;
> +}
> +
> +int prog_scope::id() const
> +{
> + return scope_id;
> +}
> +
> +int prog_scope::begin() const
> +{
> + return scope_begin;
> +}
> +
> +int prog_scope::end() const
> +{
> + return scope_end;
> +}
> +
> +void prog_scope::set_previous_case_scope(prog_scope * prev)
> +{
> + previous_case_scope = prev;
> +}
> +
> +void prog_scope::set_end(int end)
> +{
> + if (scope_end == -1) {
> + scope_end = end;
> + if (previous_case_scope)
> + previous_case_scope->set_end(end);
> + }
> +}
> +
> +void prog_scope::set_continue_line(int line)
> +{
> + if (scope_type == sct_loop) {
> + loop_cont_line = line;
> + } else {
> + if (parent_scope)
> + parent()->set_continue_line(line);
> + }
> +}
> +
> +int prog_scope::loop_continue_line() const
> +{
> + return loop_cont_line;
> +}
> +
> +temp_access::temp_access():
> + last_read_scope(nullptr),
> + first_read_scope(nullptr),
> + first_write_scope(nullptr),
> + first_dominant_write(-1),
> + last_read(-1),
> + last_write(-1),
> + first_read(numeric_limits<int>::max()),
> + keep_for_full_loop(false)
> +{
> +}
> +
> +void temp_access::record(int line, e_acc_type rw, prog_scope * scope)
> +{
> + if (rw == acc_read) {
> +
Again, remove empty line. Further instances of this below.
> + last_read_scope = scope;
> + last_read = line;
> +
> + if (first_read > line) {
> + first_read = line;
> + first_read_scope = scope;
> + }
> +
> + } else {
> +
> + last_write = line;
> +
> + /* If no first write is assigned check whether we deal with a case where
> + * the temp is read and written in the same instructions, because then
> + * it is not a dominant write, it may even be undefined. Hence postpone
> + * the assignment if the first write, only mark that the register was
> + * written at all by remembering a scope */
> +
Closing */ on its own line, and remove empty line.
Also, I think the comment is wrong. It should count as a dominating
write even if there's a read on the same line. So the special handling
here is wrong.
What you need to do for loop handling is to use first_read <=
first_dominating_write as a check for whether the first read occurs
before the first dominating write in program order.
In general, you need to think of every instruction / line of the program
as occurring in two phases, a "read" phase and a "write" phase. Then you
don't need special cases like this.
> + if (first_dominant_write < 0) {
> +
> + if (line != last_read || (rw == acc_write_cond_from_self))
> + first_dominant_write = line;
> +
> + first_write_scope = scope;
Should this be renamed to first_dominant_write_scope?
> + }
> +
> + if (scope->is_conditional() && scope->in_loop())
> + keep_for_full_loop = true;
This is only necessary as long as we don't have a dominant write yet, right?
> +
> + }
> +
> +}
> +
> +inline lifetime make_lifetime(int b, int e)
> +{
> + lifetime lt;
> + lt.begin = b;
> + lt.end = e;
> + return lt;
> +}
> +
> +#include <iostream>
> +using std::cerr;
Always put includes and usings at the top of a source file.
> +
> +lifetime temp_access::get_required_lifetime()
> +{
> +
> + /* this temp is only read, this is undefined
> + * behaviour, so we can use the register otherwise */
> + if (!first_write_scope)
> + return make_lifetime(-1, -1);
> +
> +
> + /* Only written to, just make sure it doesn't overlap */
> + if (!last_read_scope)
> + return make_lifetime(first_dominant_write, last_write + 1);
Should this be first_write or first_dominant_write?
(Also, the kind of whitespace problems here that I don't want to repeat
everywhere)
> +
> +
> + /* Undefined behaviour: read and write in the same instruction
> + * but never written elsewhere. Since it is written, we need to
> + * keep it nevertheless.
This doesn't actually need to be undefined behavior, depending on the
instruction. It's likely to be dead code though.
Also, the actual code below doesn't reflect the comment.
> + * In this case the first dominanat write is not recorded and we use the
> + * first read to estimate the life time. This is not minimal, since another
> + * undefined first read could have happend before the first undefined
> + * write, but we don't care, because adding yet another tracking variable
> + * to handle this rare case of undefined behaviour doesn't make sense */
> + if (first_write_scope && first_dominant_write < 0) {
> + return make_lifetime(first_read, last_write + 1);
Don't you have to expand this to the extend of the outermost loop?
> + }
> +
> + const prog_scope *target_scope = last_read_scope;
> + int enclosing_scope_depth = -1;
> +
> + /* we read before writing, conditional, and in a loop
> + * hence the value must survive the loops */
> + if ((first_read <= first_dominant_write) &&
> + first_read_scope->is_conditional() &&
> + first_read_scope->in_loop()) {
> + keep_for_full_loop = true;
> + target_scope = first_read_scope->outermost_loop();
> + }
> +
> + /* Evaluate the scope that is shared by all three, first write, and
> + * first (conditional) read before write and last read. */
What's a conditional read, and why does it matter?
> + while (enclosing_scope_depth < 0) {
Too many spaces on the left of <
> + if (target_scope->contains(*first_write_scope)) {
> + enclosing_scope_depth = target_scope->nesting_depth();
> + } else if (first_write_scope->contains(*target_scope)) {
> + target_scope = first_write_scope;
> + enclosing_scope_depth = first_write_scope->nesting_depth();
> + } else {
> + target_scope = target_scope->parent();
> + assert(target_scope);
> + }
> + }
> +
> + /* propagate the read scope to the target scope */
> + while (last_read_scope->nesting_depth() > enclosing_scope_depth) {
> + /* if the read is in a loop we need to extend the
> + * variables life time to the end of that loop
> + * because at this point it is not written in the same loop */
> + if (last_read_scope->type() == sct_loop)
> + last_read = last_read_scope->end();
> +
> + last_read_scope = last_read_scope->parent();
> + }
> +
> + /* If the first read is (conditionally) before the first write we
> + * have to keep the variable for the loop */
> + if ((first_write_scope->type() == sct_loop) &&
> + (first_read <= first_dominant_write)) {
> +
> + first_dominant_write = first_write_scope->begin();
> + int lr = first_write_scope->end();
> + if (last_read < lr)
> + last_read = lr;
> + }
> +
> + /* propagate the first_write scope to the target scope */
> + while (enclosing_scope_depth < first_write_scope->nesting_depth()) {
> +
> + /* propagate lifetime also if there was a continue/break
> + * in a loop and the write was after the continue/break inside
> + * that loop. Note that this is only needed if we move up in the
> + * scopes. */
> + if (first_write_scope->loop_continue_line() < first_dominant_write &&
> + first_write_scope->end() > first_dominant_write) {
> + keep_for_full_loop = true;
> + first_dominant_write = first_write_scope->begin();
> + int lr = first_write_scope->end();
> + if (last_read < lr)
> + last_read = lr;
> + }
> +
> + first_write_scope = first_write_scope->parent();
> +
> + /* Do the propagation again for the parent loop */
> + if (first_write_scope->type() == sct_loop) {
> + if (keep_for_full_loop || (first_read <= first_dominant_write)) {
> + first_dominant_write = first_write_scope->begin();
> + int lr = first_write_scope->end();
> + if (last_read < lr)
> + last_read = lr;
> + }
> + }
> +
> + /* if we currently don't propagate the lifetime but
> + * the enclosing scope is a conditional within a loop
> + * up to the last-read level we need to propagate,
> + * todo: to tighten the life time check whether the value
> + * is written in all consitional code path below the loop */
> + if (!keep_for_full_loop &&
> + first_write_scope->is_conditional() &&
> + first_write_scope->in_loop())
> + keep_for_full_loop = true;
> +
> + }
> +
> + /* MOvin up from a loop into a conditional we might not yet have marked
> + * life-time to scope propagation. Hence, if the conditional we just moved
> + * to is within a loop, propagate in the next round. */
> + if (last_write > last_read)
> + last_read = last_write + 1;
> +
> + /* Here we are at the same scope, all is resolved */
> + return make_lifetime(first_dominant_write, last_read);
I suspect that there are a lot of logical cleanups and simplifications
that you can achieve in this function but sticking to a straight story
of what every variable really means.
But please, first address the issue of multiple components and all the
style issues, then we can see what to about this.
> +}
> +
> +prog_scope_storage::prog_scope_storage(void *mc, int n):
> + mem_ctx(mc),
> + current_slot(0)
> +{
> + storage = ralloc_array(mem_ctx, prog_scope, n);
> +}
> +
> +prog_scope_storage::~prog_scope_storage()
> +{
> + ralloc_free(storage);
> +}
> +
> +prog_scope*
> +prog_scope_storage::create(prog_scope *p, e_scope_type type, int id,
> + int lvl, int s_begin)
> +{
> + storage[current_slot] = prog_scope(p, type, id, lvl, s_begin);
> + return &storage[current_slot++];
> +}
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h
> new file mode 100644
> index 0000000000..a4124b4659
> --- /dev/null
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright © 2017 Gert Wollny
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "st_glsl_to_tgsi_private.h"
> +
> +struct lifetime {
> + int begin;
> + int end;
Document this: which "phases" of the corresponding instruction does the
lifetime include?
Cheers,
Nicolai
> +};
> +
> +void
> +get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions,
> + int ntemps, struct lifetime *lifetimes);
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list