r600g/sb: Fix memory leak by reworking uses list (rebased)

The author is Heiko Przybyl(CC'ing), the patch is rebased on top of Bartosz Tomczyk's one per Dieter Nützel's comment.
Tested-by: Constantine Charlamov <Hi-Angel@yandex.ru>
v2: Resend the patch again through git-email. The prev. rebase was sent
through Thunderbird, which screwed up tab characters, making the patch
not apply.

--------------
When fixing the stalls on evergreen I introduced leaking of the useinfo
structure(s). Sorry. Instead of allocating a new object to hold 3 values
where only one is actually used, rework the list to just store the node
pointer. Thus no allocating and deallocation is needed. Since use_info
and use_kind aren't used anywhere, drop them and reduce code complexity.
This might also save some small amount of cycles.

Thanks to Bartosz Tomczyk for finding the bug.

Reported-by: Bartosz Tomczyk <bartosz.tomczyk86 at gmail.com <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>>
Signed-off-by: Heiko Przybyl <lil_tux at web.de <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>>
Supersedes: https://patchwork.freedesktop.org/patch/135852
Signed-off-by: Marek Olšák <marek.olsak@amd.com>
Tested-by: Dieter Nützel <Dieter@nuetzel-hh.de>

diff --git a/src/gallium/drivers/r600/sb/sb_def_use.cpp b/src/gallium/drivers/r600/sb/sb_def_use.cpp
index a512d9208659d6f97926b780ea9c5b47f2d58991..68ab4ca26c06e80e5c169d98c3dc215c7822b519 100644 (file)
--- a/src/gallium/drivers/r600/sb/sb_def_use.cpp
+++ b/src/gallium/drivers/r600/sb/sb_def_use.cpp
@@ -106,58 +106,51 @@ void def_use::process_defs(node *n, vvec &vv, bool arr_def) {
 }
 
 void def_use::process_uses(node* n) {
-       unsigned k = 0;
-
-       for (vvec::iterator I = n->src.begin(), E = n->src.end(); I != E;
-                       ++I, ++k) {
+       for (vvec::iterator I = n->src.begin(), E = n->src.end(); I != E; ++I) {
                value *v = *I;
                if (!v || v->is_readonly())
                        continue;
 
                if (v->is_rel()) {
                        if (!v->rel->is_readonly())
-                               v->rel->add_use(n, UK_SRC_REL, k);
+                               v->rel->add_use(n);
 
-                       unsigned k2 = 0;
                        for (vvec::iterator I = v->muse.begin(), E = v->muse.end();
-                                       I != E; ++I, ++k2) {
+                                       I != E; ++I) {
                                value *v = *I;
                                if (!v)
                                        continue;
 
-                               v->add_use(n, UK_MAYUSE, k2);
+                               v->add_use(n);
                        }
                } else
-                       v->add_use(n, UK_SRC, k);
+                       v->add_use(n);
        }
 
-       k = 0;
-       for (vvec::iterator I = n->dst.begin(), E = n->dst.end(); I != E;
-                       ++I, ++k) {
+       for (vvec::iterator I = n->dst.begin(), E = n->dst.end(); I != E; ++I) {
                value *v = *I;
                if (!v || !v->is_rel())
                        continue;
 
                if (!v->rel->is_readonly())
-                       v->rel->add_use(n, UK_DST_REL, k);
-               unsigned k2 = 0;
+                       v->rel->add_use(n);
                for (vvec::iterator I = v->muse.begin(), E = v->muse.end();
-                               I != E; ++I, ++k2) {
+                               I != E; ++I) {
                        value *v = *I;
                        if (!v)
                                continue;
 
-                       v->add_use(n, UK_MAYDEF, k2);
+                       v->add_use(n);
                }
        }
 
        if (n->pred)
-               n->pred->add_use(n, UK_PRED, 0);
+               n->pred->add_use(n);
 
        if (n->type == NT_IF) {
                if_node *i = static_cast<if_node*>(n);
                if (i->cond)
-                       i->cond->add_use(i, UK_COND, 0);
+                       i->cond->add_use(i);
        }
 }
 

diff --git a/src/gallium/drivers/r600/sb/sb_gcm.cpp b/src/gallium/drivers/r600/sb/sb_gcm.cpp
index 9c75389ada0727b62185b912c238fadb6e2aea3f..7b43a32818e5ea17a42041c3f25fc61b19b07967 100644 (file)
--- a/src/gallium/drivers/r600/sb/sb_gcm.cpp
+++ b/src/gallium/drivers/r600/sb/sb_gcm.cpp
@@ -200,27 +200,27 @@ void gcm::td_release_val(value *v) {
        );
 
        for (uselist::iterator I = v->uses.begin(), E = v->uses.end(); I != E; ++I) {
-               use_info *u = *I;
-               if (u->op->parent != &pending) {
+               node *op = *I;
+               if (op->parent != &pending) {
                        continue;
                }
 
                GCM_DUMP(
                        sblog << "td    used in ";
-                       dump::dump_op(u->op);
+                       dump::dump_op(op);
                        sblog << "\n";
                );
 
-               assert(uses[u->op] > 0);
-               if (--uses[u->op] == 0) {
+               assert(uses[op] > 0);
+               if (--uses[op] == 0) {
                        GCM_DUMP(
                                sblog << "td        released : ";
-                               dump::dump_op(u->op);
+                               dump::dump_op(op);
                                sblog << "\n";
                        );
 
-                       pending.remove_node(u->op);
-                       ready.push_back(u->op);
+                       pending.remove_node(op);
+                       ready.push_back(op);
                }
        }
 

diff --git a/src/gallium/drivers/r600/sb/sb_ir.h b/src/gallium/drivers/r600/sb/sb_ir.h
index 74c0549a8139f9e0a525bb81df85c4e742d63319..ec973e7bfc21ba63b192b36d9419d62d01c2f886 100644 (file)
--- a/src/gallium/drivers/r600/sb/sb_ir.h
+++ b/src/gallium/drivers/r600/sb/sb_ir.h
@@ -435,26 +435,7 @@ sb_ostream& operator << (sb_ostream &o, value &v);
 
 typedef uint32_t value_hash;
 
-enum use_kind {
-       UK_SRC,
-       UK_SRC_REL,
-       UK_DST_REL,
-       UK_MAYDEF,
-       UK_MAYUSE,
-       UK_PRED,
-       UK_COND
-};
-
-struct use_info {
-       node *op;
-       use_kind kind;
-       int arg;
-
-       use_info(node *n, use_kind kind, int arg)
-               : op(n), kind(kind), arg(arg) {}
-};
-
-typedef std::list< use_info * > uselist;
+typedef std::list< node * > uselist;
 
 enum constraint_kind {
        CK_SAME_REG,
@@ -585,7 +566,7 @@ public:
                                && literal_value != literal(1.0);
        }
 
-       void add_use(node *n, use_kind kind, int arg);
+       void add_use(node *n);
        void remove_use(const node *n);
 
        value_hash hash();

diff --git a/src/gallium/drivers/r600/sb/sb_valtable.cpp b/src/gallium/drivers/r600/sb/sb_valtable.cpp
index d31a1b76d58bd10697adb7c51b812cc197d97934..a85537c2ad67282ec2e30c5cecd70e66ef981906 100644 (file)
--- a/src/gallium/drivers/r600/sb/sb_valtable.cpp
+++ b/src/gallium/drivers/r600/sb/sb_valtable.cpp
@@ -212,21 +212,20 @@ void value_table::get_values(vvec& v) {
        }
 }
 
-void value::add_use(node* n, use_kind kind, int arg) {
+void value::add_use(node* n) {
        if (0) {
        sblog << "add_use ";
        dump::dump_val(this);
        sblog << "   =>  ";
        dump::dump_op(n);
-       sblog << "     kind " << kind << "    arg " << arg << "\n";
        }
-       uses.push_back(new use_info(n, kind, arg));
+       uses.push_back(n);
 }
 
 struct use_node_comp {
        explicit use_node_comp(const node *n) : n(n) {}
-       bool operator() (const use_info *u) {
-               return u->op->hash() == n->hash();
+       bool operator() (const node *o) {
+               return o->hash() == n->hash();
        }
 
        private:
@@ -239,9 +238,7 @@ void value::remove_use(const node *n) {
 
        if (it != uses.end())
        {
-               // TODO assert((*it)->kind == kind) ?
-               // TODO assert((*it)->arg == arg) ?
-               delete *it;
+               // We only ever had a pointer, so don't delete it here
                uses.erase(it);
        }
 }
@@ -291,12 +288,8 @@ bool value::is_prealloc() {
 }
 
 void value::delete_uses() {
-       for (uselist::iterator it = uses.begin(); it != uses.end(); ++it)
-       {
-               delete *it;
-       }
-
-       uses.clear();
+       // We only ever had pointers, so don't delete them here
+       uses.erase(uses.begin(), uses.end());
 }
 
 void ra_constraint::update_values() {