From 6b379f9fe87796795be8a7268dc993427d48d5ee Mon Sep 17 00:00:00 2001
From: James Roseborough <james.roseborough@luaj.org>
Date: Tue, 10 Aug 2010 14:55:32 +0000
Subject: [PATCH] Improve bytecode generation.

---
 src/jse/org/luaj/vm2/luajc/JavaBuilder.java | 41 +++++++++++++++------
 src/jse/org/luaj/vm2/luajc/JavaGen.java     |  1 -
 src/jse/org/luaj/vm2/luajc/ProtoInfo.java   | 13 ++++---
 test/junit/org/luaj/vm2/FragmentsTest.java  | 11 ++++++
 4 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/src/jse/org/luaj/vm2/luajc/JavaBuilder.java b/src/jse/org/luaj/vm2/luajc/JavaBuilder.java
index 54e897e2..c4a5bcbd 100644
--- a/src/jse/org/luaj/vm2/luajc/JavaBuilder.java
+++ b/src/jse/org/luaj/vm2/luajc/JavaBuilder.java
@@ -197,7 +197,9 @@ public class JavaBuilder {
 
 		// create the fields
 		for ( int i=0; i<p.nups; i++ ) {
-			FieldGen fg = new FieldGen(0, TYPE_LOCALUPVALUE, upvalueName(i), cp);
+			boolean isrw = pi.isReadWriteUpvalue( pi.upvals[i] ); 
+			Type uptype = isrw? TYPE_LOCALUPVALUE: TYPE_LUAVALUE;
+			FieldGen fg = new FieldGen(0, uptype, upvalueName(i), cp);
 			cg.addField(fg.getField());
 		}
 		
@@ -395,18 +397,29 @@ public class JavaBuilder {
 	}
 	
 	public void loadUpvalue(int upindex) {
+		boolean isrw = pi.isReadWriteUpvalue( pi.upvals[upindex] ); 
 		append(InstructionConstants.THIS);
-		append(factory.createFieldAccess(classname, upvalueName(upindex), TYPE_LOCALUPVALUE, Constants.GETFIELD));
-		append(new PUSH(cp,0));
-		append(InstructionConstants.AALOAD);
+		if ( isrw ) {
+			append(factory.createFieldAccess(classname, upvalueName(upindex), TYPE_LOCALUPVALUE, Constants.GETFIELD));
+			append(new PUSH(cp,0));
+			append(InstructionConstants.AALOAD);
+		} else {
+			append(factory.createFieldAccess(classname, upvalueName(upindex), TYPE_LUAVALUE, Constants.GETFIELD));
+		}
 	}
 
 	public void storeUpvalue(int pc, int upindex, int slot) {
+		boolean isrw = pi.isReadWriteUpvalue( pi.upvals[upindex] ); 
 		append(InstructionConstants.THIS);
-		append(factory.createFieldAccess(classname, upvalueName(upindex), TYPE_LOCALUPVALUE, Constants.GETFIELD));
-		append(new PUSH(cp,0));
-		loadLocal(pc, slot);
-		append(InstructionConstants.AASTORE);
+		if ( isrw ) {
+			append(factory.createFieldAccess(classname, upvalueName(upindex), TYPE_LOCALUPVALUE, Constants.GETFIELD));
+			append(new PUSH(cp,0));
+			loadLocal(pc, slot);
+			append(InstructionConstants.AASTORE);
+		} else {
+			loadLocal(pc, slot);
+			append(factory.createFieldAccess(classname, upvalueName(upindex), TYPE_LUAVALUE, Constants.PUTFIELD));
+		}
 	}
 
 	
@@ -596,18 +609,22 @@ public class JavaBuilder {
 	}
 
 	public void closureInitUpvalueFromUpvalue(String protoname, int newup, int upindex) {
+		boolean isrw = pi.isReadWriteUpvalue( pi.upvals[upindex] ); 
+		Type uptype = isrw? TYPE_LOCALUPVALUE: TYPE_LUAVALUE;
 		String srcname = upvalueName(upindex);
 		String destname = upvalueName(newup);
 		append(InstructionConstants.THIS);
-		append(factory.createFieldAccess(classname, srcname, TYPE_LOCALUPVALUE, Constants.GETFIELD));
-		append(factory.createFieldAccess(protoname, destname, TYPE_LOCALUPVALUE, Constants.PUTFIELD));
+		append(factory.createFieldAccess(classname, srcname, uptype, Constants.GETFIELD));
+		append(factory.createFieldAccess(protoname, destname, uptype, Constants.PUTFIELD));
 	}
 
 	public void closureInitUpvalueFromLocal(String protoname, int newup, int pc, int srcslot) {
+		boolean isrw = pi.isReadWriteUpvalue( pi.vars[srcslot][pc].upvalue ); 
+		Type uptype = isrw? TYPE_LOCALUPVALUE: TYPE_LUAVALUE;
 		String destname = upvalueName(newup);
-		int index = findSlotIndex( srcslot, true );
+		int index = findSlotIndex( srcslot, isrw );
 		append(new ALOAD(index));
-		append(factory.createFieldAccess(protoname, destname, TYPE_LOCALUPVALUE, Constants.PUTFIELD));
+		append(factory.createFieldAccess(protoname, destname, uptype, Constants.PUTFIELD));
 	}
 	
 	private Map<LuaValue,String> constants = new HashMap<LuaValue,String>();
diff --git a/src/jse/org/luaj/vm2/luajc/JavaGen.java b/src/jse/org/luaj/vm2/luajc/JavaGen.java
index b2a7973c..9aa40af2 100644
--- a/src/jse/org/luaj/vm2/luajc/JavaGen.java
+++ b/src/jse/org/luaj/vm2/luajc/JavaGen.java
@@ -317,7 +317,6 @@ public class JavaGen {
 				builder.dup();
 				builder.dup();
 				builder.storeLocal(pc, a);
-//				builder.createUpvalues(pc, a+3, 1);
 				builder.storeLocal(pc, a+3);
 				builder.loadLocal(pc, a+1); // limit
 				builder.loadLocal(pc, a+2); // step
diff --git a/src/jse/org/luaj/vm2/luajc/ProtoInfo.java b/src/jse/org/luaj/vm2/luajc/ProtoInfo.java
index d8f7bcdc..94af7867 100644
--- a/src/jse/org/luaj/vm2/luajc/ProtoInfo.java
+++ b/src/jse/org/luaj/vm2/luajc/ProtoInfo.java
@@ -434,14 +434,12 @@ public class ProtoInfo {
 
 	public boolean isUpvalueAssign(int pc, int slot) {
 		VarInfo v = pc<0? params[slot]: vars[slot][pc];
-//		return v.upvalue != null && v.upvalue.rw;
-		return v != null && v.upvalue != null;
+		return v != null && v.upvalue != null && v.upvalue.rw;
 	}
 
 	public boolean isUpvalueCreate(int pc, int slot) {
 		VarInfo v = pc<0? params[slot]: vars[slot][pc];
-//		return v.upvalue != null && v.upvalue.rw && v.allocupvalue && pc == v.pc;
-		return v != null && v.upvalue != null && v.allocupvalue && pc == v.pc;
+		return v != null && v.upvalue != null && v.upvalue.rw && v.allocupvalue && pc == v.pc;
 	}
 
 	public boolean isUpvalueRefer(int pc, int slot) {
@@ -449,12 +447,15 @@ public class ProtoInfo {
 		if ( pc >= 0 && vars[slot][pc] != null && vars[slot][pc].pc == pc )
 			pc -= 1;
 		VarInfo v = pc<0? params[slot]: vars[slot][pc];
-//		return v.upvalue != null && v.upvalue.rw;
-		return v != null && v.upvalue != null;
+		return v != null && v.upvalue != null && v.upvalue.rw;
 	}
 
 	public boolean isInitialValueUsed(int slot) {
 		VarInfo v = params[slot];
 		return v.isreferenced;
 	}
+
+	public boolean isReadWriteUpvalue(UpvalInfo u) {
+		return u.rw;
+	}
 }
diff --git a/test/junit/org/luaj/vm2/FragmentsTest.java b/test/junit/org/luaj/vm2/FragmentsTest.java
index 788be216..4ec860c9 100644
--- a/test/junit/org/luaj/vm2/FragmentsTest.java
+++ b/test/junit/org/luaj/vm2/FragmentsTest.java
@@ -442,5 +442,16 @@ public class FragmentsTest extends TestSuite {
 					"end\n" +
 					"return bar()[1]");
 		}
+		
+		public void testReadOnlyAndReadWriteUpvalues() {
+			runFragment( LuaValue.varargsOf( new LuaValue[] { LuaValue.valueOf(333), LuaValue.valueOf(222) } ),
+					"local a = 111\n" +
+					"local b = 222\n" +
+					"local c = function()\n"+
+					"	a = a + b\n" +
+					"	return a,b\n"+
+					"end\n" +
+					"return c()\n" );
+		}
 	}
 }