From 84249fde93e6c45e7e31ca2d641b3ac09d3e6c5c Mon Sep 17 00:00:00 2001
From: Nicolas Capens <capn@google.com>
Date: Thu, 9 Nov 2017 11:20:51 -0500
Subject: [PATCH] Don't needlessly traverse the left hand side of assignments.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Previously we processed assignments in PostVisit, i.e. after both the
left and right side had been traversed. This produces temporaries for
the left hand side, which we don't use since we want to assign to the
lvalue. So instead we can explicitly traverse the right hand side, and
for the left hand side only traverse indirect indexing expressions.

Change-Id: I9ec0596a9c256921b65a9f70428d950959f66aa0
Reviewed-on: https://swiftshader-review.googlesource.com/13630
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
---
 src/OpenGL/compiler/OutputASM.cpp | 74 +++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 35 deletions(-)

diff --git a/src/OpenGL/compiler/OutputASM.cpp b/src/OpenGL/compiler/OutputASM.cpp
index 2f5a04f5f..56fde3dba 100644
--- a/src/OpenGL/compiler/OutputASM.cpp
+++ b/src/OpenGL/compiler/OutputASM.cpp
@@ -507,32 +507,30 @@ namespace glsl
 		switch(node->getOp())
 		{
 		case EOpAssign:
-			if(visit == PostVisit)
-			{
-				assignLvalue(left, right);
-				copy(result, right);
-			}
-			break;
+			assert(visit == PreVisit);
+			right->traverse(this);
+			assignLvalue(left, right);
+			copy(result, right);
+			return false;
 		case EOpInitialize:
-			if(visit == PostVisit)
-			{
-				copy(left, right);
-			}
-			break;
+			assert(visit == PreVisit);
+			right->traverse(this);
+			copy(left, right);
+			return false;
 		case EOpMatrixTimesScalarAssign:
-			if(visit == PostVisit)
+			assert(visit == PreVisit);
+			right->traverse(this);
+			for(int i = 0; i < leftType.getNominalSize(); i++)
 			{
-				for(int i = 0; i < leftType.getNominalSize(); i++)
-				{
-					emit(sw::Shader::OPCODE_MUL, result, i, left, i, right);
-				}
-
-				assignLvalue(left, result);
+				emit(sw::Shader::OPCODE_MUL, result, i, left, i, right);
 			}
-			break;
+
+			assignLvalue(left, result);
+			return false;
 		case EOpVectorTimesMatrixAssign:
-			if(visit == PostVisit)
+			assert(visit == PreVisit);
 			{
+				right->traverse(this);
 				int size = leftType.getNominalSize();
 
 				for(int i = 0; i < size; i++)
@@ -543,10 +541,11 @@ namespace glsl
 
 				assignLvalue(left, result);
 			}
-			break;
+			return false;
 		case EOpMatrixTimesMatrixAssign:
-			if(visit == PostVisit)
+			assert(visit == PreVisit);
 			{
+				right->traverse(this);
 				int dim = leftType.getNominalSize();
 
 				for(int i = 0; i < dim; i++)
@@ -563,7 +562,7 @@ namespace glsl
 
 				assignLvalue(left, result);
 			}
-			break;
+			return false;
 		case EOpIndexDirect:
 			if(visit == PostVisit)
 			{
@@ -2311,9 +2310,8 @@ namespace glsl
 
 	void OutputASM::assignLvalue(TIntermTyped *dst, TIntermTyped *src)
 	{
-		if(src &&
-			((src->isVector() && (!dst->isVector() || (src->getNominalSize() != dst->getNominalSize()))) ||
-			 (src->isMatrix() && (!dst->isMatrix() || (src->getNominalSize() != dst->getNominalSize()) || (src->getSecondarySize() != dst->getSecondarySize())))))
+		if((src->isVector() && (!dst->isVector() || (src->getNominalSize() != dst->getNominalSize()))) ||
+		   (src->isMatrix() && (!dst->isMatrix() || (src->getNominalSize() != dst->getNominalSize()) || (src->getSecondarySize() != dst->getSecondarySize()))))
 		{
 			return mContext.error(src->getLine(), "Result type should match the l-value type in compound assignment", src->isVector() ? "vector" : "matrix");
 		}
@@ -2337,21 +2335,25 @@ namespace glsl
 		}
 		else
 		{
-			for(int offset = 0; offset < dst->totalRegisterCount(); offset++)
+			Instruction *mov1 = new Instruction(sw::Shader::OPCODE_MOV);
+
+			Temporary address(this);
+			int swizzle = lvalue(mov1->dst, address, dst);
+
+			argument(mov1->src[0], src);
+			mov1->src[0].swizzle = swizzleSwizzle(mov1->src[0].swizzle, swizzle);
+
+			shader->append(mov1);
+
+			for(int offset = 1; offset < dst->totalRegisterCount(); offset++)
 			{
 				Instruction *mov = new Instruction(sw::Shader::OPCODE_MOV);
 
-				Temporary address(this);
-				int swizzle = lvalue(mov->dst, address, dst);
+				mov->dst = mov1->dst;
 				mov->dst.index += offset;
-
-				if(offset > 0)
-				{
-					mov->dst.mask = writeMask(dst, offset);
-				}
+				mov->dst.mask = writeMask(dst, offset);
 
 				argument(mov->src[0], src, offset);
-				mov->src[0].swizzle = swizzleSwizzle(mov->src[0].swizzle, swizzle);
 
 				shader->append(mov);
 			}
@@ -2402,6 +2404,8 @@ namespace glsl
 				break;
 			case EOpIndexIndirect:
 				{
+					right->traverse(this);
+
 					if(left->isRegister())
 					{
 						// Requires INSERT instruction (handled by calling function)
-- 
2.11.0