From: Mikko Rasa <tdb@tdb.fi>
Date: Sun, 14 Mar 2021 14:11:07 +0000 (+0200)
Subject: Make variable renaming while inlining more robust
X-Git-Url: http://git.tdb.fi/?a=commitdiff_plain;h=0388083;p=libs%2Fgl.git

Make variable renaming while inlining more robust

It now prevents accidentally hiding global variables too.
---

diff --git a/source/glsl/optimize.cpp b/source/glsl/optimize.cpp
index c8c9041f..c4118dee 100644
--- a/source/glsl/optimize.cpp
+++ b/source/glsl/optimize.cpp
@@ -74,6 +74,10 @@ const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &ta
 {
 	target_block = &tgt_blk;
 	source_func = &src;
+	remap_prefix = source_func->name;
+
+	vector<RefPtr<Statement> > inlined;
+	inlined.reserve(src.body.body.size());
 	for(NodeList<Statement>::iterator i=src.body.body.begin(); i!=src.body.body.end(); ++i)
 	{
 		r_inlined_statement = 0;
@@ -81,11 +85,18 @@ const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &ta
 		if(!r_inlined_statement)
 			r_inlined_statement = (*i)->clone();
 
-		SetFlag set_remap(remap_names);
+		SetForScope<unsigned> set_remap(remap_names, 2);
 		r_inlined_statement->visit(*this);
-		tgt_blk.body.insert(ins_pt, r_inlined_statement);
+		inlined.push_back(r_inlined_statement);
 	}
 
+	SetForScope<unsigned> set_remap(remap_names, 1);
+	SetForScope<string> set_prefix(remap_prefix, target_func.name);
+	variable_map.clear();
+	target_func.visit(*this);
+
+	tgt_blk.body.insert(ins_pt, inlined.begin(), inlined.end());
+
 	NodeReorderer().apply(stage, target_func, dependencies);
 
 	return r_result_name;
@@ -102,7 +113,11 @@ void InlineContentInjector::visit(VariableReference &var)
 	else if(var.declaration)
 	{
 		SetFlag set_deps(deps_only);
-		dependencies.insert(var.declaration);
+		if(!variable_map.count(var.name))
+		{
+			dependencies.insert(var.declaration);
+			referenced_names.insert(var.name);
+		}
 		var.declaration->visit(*this);
 	}
 }
@@ -113,6 +128,7 @@ void InlineContentInjector::visit(InterfaceBlockReference &iface)
 	{
 		SetFlag set_deps(deps_only);
 		dependencies.insert(iface.declaration);
+		referenced_names.insert(iface.name);
 		iface.declaration->visit(*this);
 	}
 }
@@ -120,7 +136,10 @@ void InlineContentInjector::visit(InterfaceBlockReference &iface)
 void InlineContentInjector::visit(FunctionCall &call)
 {
 	if(!remap_names && call.declaration)
+	{
 		dependencies.insert(call.declaration);
+		referenced_names.insert(call.name);
+	}
 	TraversingVisitor::visit(call);
 }
 
@@ -128,28 +147,29 @@ void InlineContentInjector::visit(VariableDeclaration &var)
 {
 	TraversingVisitor::visit(var);
 
-	if(var.type_declaration)
+	if(remap_names)
+	{
+		if(remap_names==2 || referenced_names.count(var.name))
+		{
+			string mapped_name = get_unused_variable_name(*target_block, var.name, remap_prefix);
+			variable_map[var.name] = &var;
+			var.name = mapped_name;
+		}
+	}
+	else if(var.type_declaration)
 	{
 		SetFlag set_deps(deps_only);
 		dependencies.insert(var.type_declaration);
+		referenced_names.insert(var.type_declaration->name);
 		var.type_declaration->visit(*this);
 	}
-
-	if(!remap_names && !deps_only)
-	{
-		RefPtr<VariableDeclaration> inlined_var = var.clone();
-		inlined_var->name = get_unused_variable_name(*target_block, var.name, source_func->name);
-		r_inlined_statement = inlined_var;
-
-		variable_map[var.name] = inlined_var.get();
-	}
 }
 
 void InlineContentInjector::visit(Return &ret)
 {
 	TraversingVisitor::visit(ret);
 
-	if(ret.expression)
+	if(!remap_names && ret.expression)
 	{
 		/* Create a new variable to hold the return value of the inlined
 		function. */
diff --git a/source/glsl/optimize.h b/source/glsl/optimize.h
index ab954b58..8a888e15 100644
--- a/source/glsl/optimize.h
+++ b/source/glsl/optimize.h
@@ -43,10 +43,12 @@ private:
 	FunctionDeclaration *source_func;
 	Block *target_block;
 	std::map<std::string, VariableDeclaration *> variable_map;
-	bool remap_names;
+	std::string remap_prefix;
+	unsigned remap_names;
 	bool deps_only;
 	RefPtr<Statement> r_inlined_statement;
 	std::set<Node *> dependencies;
+	std::set<std::string> referenced_names;
 	std::string r_result_name;
 
 public:
diff --git a/source/glsl/syntax.cpp b/source/glsl/syntax.cpp
index cb50fee1..c94e2e27 100644
--- a/source/glsl/syntax.cpp
+++ b/source/glsl/syntax.cpp
@@ -472,8 +472,14 @@ string get_unused_variable_name(const Block &block, const string &base, const st
 	bool prefixed = false;
 	unsigned number = 1;
 	unsigned size_without_number = name.size();
-	while(block.variables.count(name))
+	while(1)
 	{
+		bool unused = true;
+		for(const Block *b=&block; (unused && b); b=b->parent)
+			unused = !b->variables.count(name);
+		if(unused)
+			return name;
+
 		if(!prefixed && !prefix_hint.empty())
 		{
 			if(name.front()!='_')
@@ -491,8 +497,6 @@ string get_unused_variable_name(const Block &block, const string &base, const st
 			++number;
 		}
 	}
-
-	return name;
 }
 
 } // namespace SL
diff --git a/tests/glsl/function_inline_global_name_conflict.glsl b/tests/glsl/function_inline_global_name_conflict.glsl
new file mode 100644
index 00000000..8ebbfb46
--- /dev/null
+++ b/tests/glsl/function_inline_global_name_conflict.glsl
@@ -0,0 +1,28 @@
+#pragma MSP stage(vertex)
+layout(location=0) in vec4 position;
+layout(location=1) in float scale;
+layout(location=2) in float size;
+float func()
+{
+	float scale = size*2.0;
+	return scale*scale;
+}
+void main()
+{
+	float size = scale+1.0;
+	float s = func()+1.0;
+	gl_Position = position*size*size*s*s;
+}
+
+/* Expected output: vertex
+layout(location=0) in vec4 position;
+layout(location=1) in float scale;
+layout(location=2) in float size;
+void main()
+{
+	float _main_size = scale+1.0;
+	float _func_scale = size*2.0;
+	float s = _func_scale*_func_scale+1.0;
+	gl_Position = position*_main_size*_main_size*s*s;
+}
+*/