Skip to content

Commit

Permalink
Move variable slotting to the IR tree (#51452)
Browse files Browse the repository at this point in the history
Currently, variable slotting (a variable's index on the ASM stack) is 
generated by the user tree during semantic checking. With the split it no 
longer makes sense to have the user tree generate a variable's slot. This PR 
moves variable slot allocation to the IR tree using the class ScopeTable. This 
class holds variables currently in scope and calculates an appropriate slot 
for them. This also allows for optimization phases to occur on the IR tree 
because now variables can be injected without having to update slotting.
  • Loading branch information
jdconrad authored Jan 28, 2020
1 parent 4630c8d commit 9d2c579
Show file tree
Hide file tree
Showing 72 changed files with 655 additions and 360 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.painless.lookup.PainlessCast;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.elasticsearch.painless.lookup.def;
import org.elasticsearch.painless.symbol.ScopeTable;

public class AssignmentNode extends BinaryNode {

Expand Down Expand Up @@ -113,7 +114,7 @@ public PainlessCast getBack() {
/* ---- end node data ---- */

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
methodWriter.writeDebugInfo(location);

// For the case where the assignment represents a String concatenation
Expand All @@ -127,18 +128,19 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
catElementStackSize = methodWriter.writeNewStrings();
}

getLeftNode().setup(classWriter, methodWriter, globals); // call the setup method on the lhs to prepare for a load/store operation
// call the setup method on the lhs to prepare for a load/store operation
getLeftNode().setup(classWriter, methodWriter, globals, scopeTable);

if (cat) {
// Handle the case where we are doing a compound assignment
// representing a String concatenation.

methodWriter.writeDup(getLeftNode().accessElementCount(), catElementStackSize); // dup the top element and insert it
// before concat helper on stack
getLeftNode().load(classWriter, methodWriter, globals); // read the current lhs's value
getLeftNode().load(classWriter, methodWriter, globals, scopeTable); // read the current lhs's value
methodWriter.writeAppendStrings(getLeftNode().getExpressionType()); // append the lhs's value using the StringBuilder

getRightNode().write(classWriter, methodWriter, globals); // write the bytecode for the rhs
getRightNode().write(classWriter, methodWriter, globals, scopeTable); // write the bytecode for the rhs

// check to see if the rhs has already done a concatenation
if (getRightNode() instanceof BinaryMathNode == false || ((BinaryMathNode)getRightNode()).getCat() == false) {
Expand All @@ -156,14 +158,14 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
}

// store the lhs's value from the stack in its respective variable/field/array
getLeftNode().store(classWriter, methodWriter, globals);
getLeftNode().store(classWriter, methodWriter, globals, scopeTable);
} else if (operation != null) {
// Handle the case where we are doing a compound assignment that
// does not represent a String concatenation.

methodWriter.writeDup(getLeftNode().accessElementCount(), 0); // if necessary, dup the previous lhs's value
// to be both loaded from and stored to
getLeftNode().load(classWriter, methodWriter, globals); // load the current lhs's value
getLeftNode().load(classWriter, methodWriter, globals, scopeTable); // load the current lhs's value

if (read && post) {
// dup the value if the lhs is also read from and is a post increment
Expand All @@ -173,7 +175,7 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals

methodWriter.writeCast(there); // if necessary cast the current lhs's value
// to the promotion type between the lhs and rhs types
getRightNode().write(classWriter, methodWriter, globals); // write the bytecode for the rhs
getRightNode().write(classWriter, methodWriter, globals, scopeTable); // write the bytecode for the rhs

// XXX: fix these types, but first we need def compound assignment tests.
// its tricky here as there are possibly explicit casts, too.
Expand All @@ -194,11 +196,11 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
}

// store the lhs's value from the stack in its respective variable/field/array
getLeftNode().store(classWriter, methodWriter, globals);
getLeftNode().store(classWriter, methodWriter, globals, scopeTable);
} else {
// Handle the case for a simple write.

getRightNode().write(classWriter, methodWriter, globals); // write the bytecode for the rhs rhs
getRightNode().write(classWriter, methodWriter, globals, scopeTable); // write the bytecode for the rhs rhs

if (read) {
// dup the value if the lhs is also read from
Expand All @@ -207,7 +209,7 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
}

// store the lhs's value from the stack in its respective variable/field/array
getLeftNode().store(classWriter, methodWriter, globals);
getLeftNode().store(classWriter, methodWriter, globals, scopeTable);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.painless.WriterConstants;
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
import org.elasticsearch.painless.lookup.def;
import org.elasticsearch.painless.symbol.ScopeTable;

import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -98,21 +99,21 @@ public void setLocation(Location location) {
/* ---- end node data ---- */

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
methodWriter.writeDebugInfo(location);

if (getBinaryType() == String.class && operation == Operation.ADD) {
if (cat == false) {
methodWriter.writeNewStrings();
}

getLeftNode().write(classWriter, methodWriter, globals);
getLeftNode().write(classWriter, methodWriter, globals, scopeTable);

if (getLeftNode() instanceof BinaryMathNode == false || ((BinaryMathNode)getLeftNode()).getCat() == false) {
methodWriter.writeAppendStrings(getLeftNode().getExpressionType());
}

getRightNode().write(classWriter, methodWriter, globals);
getRightNode().write(classWriter, methodWriter, globals, scopeTable);

if (getRightNode() instanceof BinaryMathNode == false || ((BinaryMathNode)getRightNode()).getCat() == false) {
methodWriter.writeAppendStrings(getRightNode().getExpressionType());
Expand All @@ -122,8 +123,8 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
methodWriter.writeToStrings();
}
} else if (operation == Operation.FIND || operation == Operation.MATCH) {
getRightNode().write(classWriter, methodWriter, globals);
getLeftNode().write(classWriter, methodWriter, globals);
getRightNode().write(classWriter, methodWriter, globals, scopeTable);
getLeftNode().write(classWriter, methodWriter, globals, scopeTable);
methodWriter.invokeVirtual(org.objectweb.asm.Type.getType(Pattern.class), WriterConstants.PATTERN_MATCHER);

if (operation == Operation.FIND) {
Expand All @@ -135,8 +136,8 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
"for type [" + getExpressionCanonicalTypeName() + "]");
}
} else {
getLeftNode().write(classWriter, methodWriter, globals);
getRightNode().write(classWriter, methodWriter, globals);
getLeftNode().write(classWriter, methodWriter, globals, scopeTable);
getRightNode().write(classWriter, methodWriter, globals, scopeTable);

if (binaryType == def.class || (shiftType != null && shiftType == def.class)) {
// def calls adopt the wanted return value. if there was a narrowing cast,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.elasticsearch.painless.ClassWriter;
import org.elasticsearch.painless.Globals;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.symbol.ScopeTable;

import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -64,11 +65,11 @@ public int getStatementCount() {
/* ---- end node data ---- */

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
for (StatementNode statementNode : statementNodes) {
statementNode.continueLabel = continueLabel;
statementNode.breakLabel = breakLabel;
statementNode.write(classWriter, methodWriter, globals);
statementNode.write(classWriter, methodWriter, globals, scopeTable);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.painless.Globals;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.Operation;
import org.elasticsearch.painless.symbol.ScopeTable;
import org.objectweb.asm.Label;
import org.objectweb.asm.Opcodes;

Expand All @@ -43,16 +44,16 @@ public Operation getOperation() {
/* ---- end node data ---- */

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
methodWriter.writeDebugInfo(location);

if (operation == Operation.AND) {
Label fals = new Label();
Label end = new Label();

getLeftNode().write(classWriter, methodWriter, globals);
getLeftNode().write(classWriter, methodWriter, globals, scopeTable);
methodWriter.ifZCmp(Opcodes.IFEQ, fals);
getRightNode().write(classWriter, methodWriter, globals);
getRightNode().write(classWriter, methodWriter, globals, scopeTable);
methodWriter.ifZCmp(Opcodes.IFEQ, fals);

methodWriter.push(true);
Expand All @@ -65,9 +66,9 @@ protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals
Label fals = new Label();
Label end = new Label();

getLeftNode().write(classWriter, methodWriter, globals);
getLeftNode().write(classWriter, methodWriter, globals, scopeTable);
methodWriter.ifZCmp(Opcodes.IFNE, tru);
getRightNode().write(classWriter, methodWriter, globals);
getRightNode().write(classWriter, methodWriter, globals, scopeTable);
methodWriter.ifZCmp(Opcodes.IFEQ, fals);

methodWriter.mark(tru);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@
import org.elasticsearch.painless.ClassWriter;
import org.elasticsearch.painless.Globals;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.symbol.ScopeTable;

public class BraceNode extends BinaryNode {

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
getLeftNode().write(classWriter, methodWriter, globals);
getRightNode().write(classWriter, methodWriter, globals);
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
getLeftNode().write(classWriter, methodWriter, globals, scopeTable);
getRightNode().write(classWriter, methodWriter, globals, scopeTable);
}

@Override
Expand All @@ -37,18 +38,18 @@ protected int accessElementCount() {
}

@Override
protected void setup(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
getLeftNode().write(classWriter, methodWriter, globals);
getRightNode().setup(classWriter, methodWriter, globals);
protected void setup(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
getLeftNode().write(classWriter, methodWriter, globals, scopeTable);
getRightNode().setup(classWriter, methodWriter, globals, scopeTable);
}

@Override
protected void load(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
getRightNode().load(classWriter, methodWriter, globals);
protected void load(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
getRightNode().load(classWriter, methodWriter, globals, scopeTable);
}

@Override
protected void store(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
getRightNode().store(classWriter, methodWriter, globals);
protected void store(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
getRightNode().store(classWriter, methodWriter, globals, scopeTable);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@
import org.elasticsearch.painless.DefBootstrap;
import org.elasticsearch.painless.Globals;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.symbol.ScopeTable;
import org.objectweb.asm.Type;

public class BraceSubDefNode extends UnaryNode {

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
setup(classWriter, methodWriter, globals);
load(classWriter, methodWriter, globals);
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
setup(classWriter, methodWriter, globals, scopeTable);
load(classWriter, methodWriter, globals, scopeTable);
}

@Override
Expand All @@ -39,16 +40,16 @@ protected int accessElementCount() {
}

@Override
protected void setup(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void setup(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
methodWriter.dup();
getChildNode().write(classWriter, methodWriter, globals);
getChildNode().write(classWriter, methodWriter, globals, scopeTable);
Type methodType = Type.getMethodType(MethodWriter.getType(
getChildNode().getExpressionType()), Type.getType(Object.class), MethodWriter.getType(getChildNode().getExpressionType()));
methodWriter.invokeDefCall("normalizeIndex", methodType, DefBootstrap.INDEX_NORMALIZE);
}

@Override
protected void load(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void load(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
methodWriter.writeDebugInfo(location);

Type methodType = Type.getMethodType(MethodWriter.getType(
Expand All @@ -57,7 +58,7 @@ protected void load(ClassWriter classWriter, MethodWriter methodWriter, Globals
}

@Override
protected void store(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void store(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
methodWriter.writeDebugInfo(location);

Type methodType = Type.getMethodType(Type.getType(void.class), Type.getType(Object.class),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,16 @@
import org.elasticsearch.painless.ClassWriter;
import org.elasticsearch.painless.Globals;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.symbol.ScopeTable;
import org.objectweb.asm.Label;
import org.objectweb.asm.Opcodes;

public class BraceSubNode extends UnaryNode {

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
setup(classWriter, methodWriter, globals);
load(classWriter, methodWriter, globals);
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
setup(classWriter, methodWriter, globals, scopeTable);
load(classWriter, methodWriter, globals, scopeTable);
}

@Override
Expand All @@ -39,8 +40,8 @@ protected int accessElementCount() {
}

@Override
protected void setup(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
getChildNode().write(classWriter, methodWriter, globals);
protected void setup(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
getChildNode().write(classWriter, methodWriter, globals, scopeTable);

Label noFlip = new Label();
methodWriter.dup();
Expand All @@ -53,13 +54,13 @@ protected void setup(ClassWriter classWriter, MethodWriter methodWriter, Globals
}

@Override
protected void load(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void load(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
methodWriter.writeDebugInfo(location);
methodWriter.arrayLoad(MethodWriter.getType(getExpressionType()));
}

@Override
protected void store(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void store(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
methodWriter.writeDebugInfo(location);
methodWriter.arrayStore(MethodWriter.getType(getExpressionType()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@
import org.elasticsearch.painless.ClassWriter;
import org.elasticsearch.painless.Globals;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.symbol.ScopeTable;

public class BreakNode extends StatementNode {

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
methodWriter.goTo(breakLabel);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@
import org.elasticsearch.painless.ClassWriter;
import org.elasticsearch.painless.Globals;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.symbol.ScopeTable;

public class CallNode extends BinaryNode {

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
getLeftNode().write(classWriter, methodWriter, globals);
getRightNode().write(classWriter, methodWriter, globals);
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
getLeftNode().write(classWriter, methodWriter, globals, scopeTable);
getRightNode().write(classWriter, methodWriter, globals, scopeTable);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.painless.DefBootstrap;
import org.elasticsearch.painless.Globals;
import org.elasticsearch.painless.MethodWriter;
import org.elasticsearch.painless.symbol.ScopeTable;
import org.objectweb.asm.Type;

import java.util.ArrayList;
Expand Down Expand Up @@ -72,11 +73,11 @@ public List<Class<?>> getTypeParameters() {
/* ---- end node data ---- */

@Override
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals) {
protected void write(ClassWriter classWriter, MethodWriter methodWriter, Globals globals, ScopeTable scopeTable) {
methodWriter.writeDebugInfo(location);

for (ExpressionNode argumentNode : getArgumentNodes()) {
argumentNode.write(classWriter, methodWriter, globals);
argumentNode.write(classWriter, methodWriter, globals, scopeTable);
}

// create method type from return value and arguments
Expand Down
Loading

0 comments on commit 9d2c579

Please sign in to comment.