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 be96517577e441d9af9563dc7e1534fdb8b50b83..1403f4a206bb3ad207b9fa88462a2b5d99f20ef3 100644 (file)
--- 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 30bc17e94bddf46ecb5b86142d729a8cd4fce73f..7f885360d78c6c18efad30852da413bb1e745896 100644 (file)
--- 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 d1468e3cc88c1db898caca337e9f396358a4041a..29df4f3d9d6d64fde1320f5b9fb049e9f7892206 100644 (file)
--- 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;