[Mesa-dev] [PATCH] r600g/sb: Fix memory leak by reworking uses list

Heiko Przybyl lil_tux at web.de
Tue Feb 7 21:30:19 UTC 2017


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>
Signed-off-by: Heiko Przybyl <lil_tux at web.de>
Supersedes: https://patchwork.freedesktop.org/patch/135852
---
 src/gallium/drivers/r600/sb/sb_def_use.cpp  | 29 +++++++++++------------------
 src/gallium/drivers/r600/sb/sb_gcm.cpp      | 16 ++++++++--------
 src/gallium/drivers/r600/sb/sb_ir.h         | 23 ++---------------------
 src/gallium/drivers/r600/sb/sb_valtable.cpp | 13 ++++++-------
 4 files changed, 27 insertions(+), 54 deletions(-)

diff --git a/src/gallium/drivers/r600/sb/sb_def_use.cpp b/src/gallium/drivers/r600/sb/sb_def_use.cpp
index a512d92086..68ab4ca26c 100644
--- 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 9c75389ada..7b43a32818 100644
--- 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 74c0549a81..ec973e7bfc 100644
--- 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 a8b7b49cd4..a85537c2ad 100644
--- 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,8 +238,7 @@ void value::remove_use(const node *n) {
 
 	if (it != uses.end())
 	{
-		// TODO assert((*it)->kind == kind) ?
-		// TODO assert((*it)->arg == arg) ?
+		// We only ever had a pointer, so don't delete it here
 		uses.erase(it);
 	}
 }
@@ -290,6 +288,7 @@ bool value::is_prealloc() {
 }
 
 void value::delete_uses() {
+	// We only ever had pointers, so don't delete them here
 	uses.erase(uses.begin(), uses.end());
 }
 
-- 
2.11.1



More information about the mesa-dev mailing list