From d5c38c181cac4da490c96123e5a88a8c06785e55 Mon Sep 17 00:00:00 2001
From: Mikko Rasa <tdb@tdb.fi>
Date: Tue, 16 Mar 2021 13:08:27 +0200
Subject: [PATCH] Use a temporary staging block in InlineContentInjector

This allows the return variable name generation to avoid other names from
the source function, such as a return variable from previous inlining.
---
 source/glsl/optimize.cpp                      | 24 +++++++++----------
 source/glsl/optimize.h                        |  3 +--
 .../glsl/function_inline_return_conflict.glsl | 24 +++++++++++++++++++
 3 files changed, 37 insertions(+), 14 deletions(-)
 create mode 100644 tests/glsl/function_inline_return_conflict.glsl

diff --git a/source/glsl/optimize.cpp b/source/glsl/optimize.cpp
index db779d1b..668f7c5e 100644
--- a/source/glsl/optimize.cpp
+++ b/source/glsl/optimize.cpp
@@ -71,12 +71,10 @@ InlineContentInjector::InlineContentInjector():
 
 const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &target_func, Block &tgt_blk, const NodeList<Statement>::iterator &ins_pt, FunctionDeclaration &src)
 {
-	target_block = &tgt_blk;
+	staging_block.parent = &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;
@@ -86,15 +84,15 @@ const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &ta
 
 		SetForScope<unsigned> set_remap(remap_names, 2);
 		r_inlined_statement->visit(*this);
-		inlined.push_back(r_inlined_statement);
+		staging_block.body.push_back(r_inlined_statement);
 	}
 
 	SetForScope<unsigned> set_remap(remap_names, 1);
 	SetForScope<string> set_prefix(remap_prefix, target_func.name);
-	variable_map.clear();
+	staging_block.variables.clear();
 	target_func.visit(*this);
 
-	tgt_blk.body.insert(ins_pt, inlined.begin(), inlined.end());
+	tgt_blk.body.splice(ins_pt, staging_block.body);
 
 	NodeReorderer().apply(stage, target_func, dependencies);
 
@@ -105,13 +103,13 @@ void InlineContentInjector::visit(VariableReference &var)
 {
 	if(remap_names)
 	{
-		map<string, VariableDeclaration *>::const_iterator i = variable_map.find(var.name);
-		if(i!=variable_map.end())
+		map<string, VariableDeclaration *>::const_iterator i = staging_block.variables.find(var.name);
+		if(i!=staging_block.variables.end())
 			var.name = i->second->name;
 	}
 	else if(var.declaration)
 	{
-		if(!variable_map.count(var.name))
+		if(!staging_block.variables.count(var.name))
 		{
 			dependencies.insert(var.declaration);
 			referenced_names.insert(var.name);
@@ -148,8 +146,10 @@ void InlineContentInjector::visit(VariableDeclaration &var)
 	{
 		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;
+			string mapped_name = get_unused_variable_name(staging_block, var.name, remap_prefix);
+			staging_block.variables[var.name] = &var;
+			if(mapped_name!=var.name)
+				staging_block.variables[mapped_name] = &var;
 			var.name = mapped_name;
 		}
 	}
@@ -169,7 +169,7 @@ void InlineContentInjector::visit(Return &ret)
 	{
 		/* Create a new variable to hold the return value of the inlined
 		function. */
-		r_result_name = get_unused_variable_name(*target_block, "_return", source_func->name);
+		r_result_name = get_unused_variable_name(staging_block, "_return", source_func->name);
 		RefPtr<VariableDeclaration> var = new VariableDeclaration;
 		var->source = ret.source;
 		var->line = ret.line;
diff --git a/source/glsl/optimize.h b/source/glsl/optimize.h
index bffc5a03..08ee082b 100644
--- a/source/glsl/optimize.h
+++ b/source/glsl/optimize.h
@@ -40,8 +40,7 @@ class InlineContentInjector: private TraversingVisitor
 {
 private:
 	FunctionDeclaration *source_func;
-	Block *target_block;
-	std::map<std::string, VariableDeclaration *> variable_map;
+	Block staging_block;
 	std::string remap_prefix;
 	unsigned remap_names;
 	RefPtr<Statement> r_inlined_statement;
diff --git a/tests/glsl/function_inline_return_conflict.glsl b/tests/glsl/function_inline_return_conflict.glsl
new file mode 100644
index 00000000..50bf1bd5
--- /dev/null
+++ b/tests/glsl/function_inline_return_conflict.glsl
@@ -0,0 +1,24 @@
+#pragma MSP stage(vertex)
+layout(location=0) in vec4 position;
+layout(location=1) in float scale;
+vec2 func()
+{
+	float _return = scale*2.0;
+	return vec2(_return, _return);
+}
+void main()
+{
+	float _return = scale+1.0;
+	gl_Position = position*func().xxyy*_return*_return;
+}
+
+/* Expected output: vertex
+layout(location=0) in vec4 position;
+layout(location=1) in float scale;
+void main()
+{
+	float _return = scale+1.0;
+	float _func_return = scale*2.0;
+	gl_Position = position*vec2(_func_return, _func_return).xxyy*_return*_return;
+}
+*/
-- 
2.43.0