From: Mikko Rasa <tdb@tdb.fi>
Date: Sat, 16 Dec 2023 20:55:47 +0000 (+0200)
Subject: Prevent inlining of an expression that got something inlined into it
X-Git-Url: https://git.tdb.fi/?a=commitdiff_plain;h=2a7662248f334dbd3300800018e1f0ec77991ffb;p=libs%2Fgl.git

Prevent inlining of an expression that got something inlined into it

It's causing some issues with converting the compiler to use unique_ptr.
This way variable names make more sense in some situations as well.
---

diff --git a/source/glsl/optimize.cpp b/source/glsl/optimize.cpp
index be965175..1403f4a2 100644
--- a/source/glsl/optimize.cpp
+++ b/source/glsl/optimize.cpp
@@ -342,6 +342,8 @@ bool ExpressionInliner::apply(Stage &s)
 				if(!u.blocked)
 				{
 					*u.reference = e.expression->clone();
+					if(u.containing_expr)
+						u.containing_expr->expression = 0;
 					any_inlined = true;
 				}
 		}
@@ -358,6 +360,7 @@ void ExpressionInliner::visit(RefPtr<Expression> &expr)
 		ExpressionUse use;
 		use.reference = &expr;
 		use.ref_scope = current_block;
+		use.containing_expr = current_expr;
 		use.blocked = access_write || r_ref_info->blocked;
 
 		if(iteration_body && !r_ref_info->trivial)
@@ -426,12 +429,25 @@ void ExpressionInliner::visit(Assignment &assign)
 		SetFlag set_write(access_write);
 		visit(assign.left);
 	}
+
+	ExpressionInfo *info = nullptr;
+	auto i = assignments.find(assign.target.declaration);
+	if(i!=assignments.end())
+	{
+		info = &expressions.emplace_back();
+		info->target = assign.target;
+		// Self-referencing assignments can't be inlined without additional work.
+		if(!assign.self_referencing)
+			info->expression = assign.right;
+		info->assign_scope = current_block;
+	}
+
 	r_oper = nullptr;
 	r_trivial = true;
+	SetForScope<ExpressionInfo *> set_expr(current_expr, info);
 	visit(assign.right);
 
-	auto i = assignments.find(assign.target.declaration);
-	if(i!=assignments.end())
+	if(info)
 	{
 		if(iteration_body && i->second && i->second->expression)
 		{
@@ -447,15 +463,8 @@ void ExpressionInliner::visit(Assignment &assign)
 			if(targets_overlap(i->first, assign.target))
 				i->second->blocked = true;
 
-		ExpressionInfo &info = expressions.emplace_back();
-		info.target = assign.target;
-		// Self-referencing assignments can't be inlined without additional work.
-		if(!assign.self_referencing)
-			info.expression = assign.right;
-		info.assign_scope = current_block;
-		info.trivial = r_trivial;
-
-		assignments[assign.target] = &info;
+		info->trivial = r_trivial;
+		assignments[assign.target] = info;
 	}
 
 	r_trivial = false;
@@ -477,32 +486,41 @@ void ExpressionInliner::visit(FunctionCall &call)
 
 void ExpressionInliner::visit(VariableDeclaration &var)
 {
+	ExpressionInfo *info = nullptr;
+	if(var.interface.empty())
+	{
+		info = &expressions.emplace_back();
+		info->target = &var;
+		/* Assume variables declared in an iteration initialization statement
+		will have their values change throughout the iteration. */
+		if(!iteration_init)
+			info->expression = var.init_expression;
+		info->assign_scope = current_block;
+	}
+
+	SetForScope<ExpressionInfo *> set_expr(current_expr, info);
 	r_oper = nullptr;
 	r_trivial = true;
 	TraversingVisitor::visit(var);
 
-	bool constant = var.constant;
-	if(constant && var.layout)
+	if(info)
 	{
-		constant = !any_of(var.layout->qualifiers.begin(), var.layout->qualifiers.end(),
-			[](const Layout::Qualifier &q){ return q.name=="constant_id"; });
-	}
+		bool constant = var.constant;
+		if(constant && var.layout)
+		{
+			constant = !any_of(var.layout->qualifiers.begin(), var.layout->qualifiers.end(),
+				[](const Layout::Qualifier &q){ return q.name=="constant_id"; });
+		}
 
-	/* Only inline global variables if they're constant and have trivial
-	initializers.  Non-constant variables could change in ways which are hard to
-	analyze and non-trivial expressions could be expensive to inline.  */
-	if((current_block->parent || (constant && r_trivial)) && var.interface.empty())
-	{
-		ExpressionInfo &info = expressions.emplace_back();
-		info.target = &var;
-		/* Assume variables declared in an iteration initialization statement
-		will have their values change throughout the iteration. */
-		if(!iteration_init)
-			info.expression = var.init_expression;
-		info.assign_scope = current_block;
-		info.trivial = r_trivial;
+		/* Block global variables from being inlined unless it's a constant with
+		a trivial initializer.  Non-constant variables could change in ways which
+		are hard to analyze and non-trivial expressions could be expensive to
+		inline. */
+		if(!current_block->parent && !(constant && r_trivial))
+			info->blocked = true;
 
-		assignments[&var] = &info;
+		info->trivial = r_trivial;
+		assignments[&var] = info;
 	}
 }
 
diff --git a/source/glsl/optimize.h b/source/glsl/optimize.h
index 30bc17e9..7f885360 100644
--- a/source/glsl/optimize.h
+++ b/source/glsl/optimize.h
@@ -107,10 +107,13 @@ Variables which are only referenced once are also inlined. */
 class ExpressionInliner: private TraversingVisitor
 {
 private:
+	struct ExpressionInfo;
+
 	struct ExpressionUse
 	{
 		RefPtr<Expression> *reference = nullptr;
 		Block *ref_scope = nullptr;
+		ExpressionInfo *containing_expr = nullptr;
 		bool blocked = false;
 	};
 
@@ -126,6 +129,7 @@ private:
 
 	std::list<ExpressionInfo> expressions;
 	std::map<Assignment::Target, ExpressionInfo *> assignments;
+	ExpressionInfo *current_expr = nullptr;
 	ExpressionInfo *r_ref_info = nullptr;
 	bool r_trivial = false;
 	bool access_read = true;
diff --git a/tests/glsl/generated_name_reuse.glsl b/tests/glsl/generated_name_reuse.glsl
index d1468e3c..29df4f3d 100644
--- a/tests/glsl/generated_name_reuse.glsl
+++ b/tests/glsl/generated_name_reuse.glsl
@@ -79,8 +79,8 @@ void main()
 	vec3 color = vec3(0.0);
 	if(use_light)
 	{
-		vec4 _return = light_position;
-		color += max(dot(normalize(_return.xyz-world_vertex.xyz*_return.w), normalize(world_normal)), 0.0);
+		vec4 light_pos = light_position;
+		color += max(dot(normalize(light_pos.xyz-world_vertex.xyz*light_pos.w), normalize(world_normal)), 0.0);
 	}
 	if(use_ambient)
 		color += ambient;