Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues with PE: move context level for non-local returns to bytecode node #210

Merged
merged 2 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public static void emitRETURNSELF(final BytecodeMethodGenContext mgenc) {
}

public static void emitRETURNNONLOCAL(final BytecodeMethodGenContext mgenc) {
emit2(mgenc, RETURN_NON_LOCAL, mgenc.getMaxContextLevel(), 0);
emit1(mgenc, RETURN_NON_LOCAL, 0);
}

public static void emitRETURNFIELD(final BytecodeMethodGenContext mgenc, final byte idx) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ private BytecodeLoopNode constructBytecodeBody(final long coord) {
BackJump[] loops = inlinedLoops.toArray(new BackJump[0]);

return new BytecodeLoopNode(bytecodes, locals.size(), literalsArr, maxStackDepth,
frameOnStackMarkerIndex, loops);
frameOnStackMarkerIndex, loops, getMaxContextLevel());
}

public byte[] getBytecodeArray() {
Expand Down
4 changes: 3 additions & 1 deletion src/trufflesom/src/trufflesom/compiler/bc/Disassembler.java
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,9 @@ public static void dumpMethod(final List<Byte> bytecodes, final String indent,
}

case RETURN_NON_LOCAL: {
Universe.errorPrintln("context: " + bytecodes.get(b + 1));
if (m != null) {
Universe.errorPrintln("context: " + m.getContextLevel());
}
break;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ public static final boolean isOneOf(final byte bytecode, final byte[] arr) {
2, // SEND
2, // SUPER_SEND
1, // RETURN_LOCAL
2, // RETURN_NON_LOCAL
1, // RETURN_NON_LOCAL
1, // RETURN_SELF

1, // RETURN_FIELD_0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,16 @@ public class BytecodeLoopNode extends NoPreEvalExprNode implements ScopeReferenc

@Children private final Node[] quickenedField;

@CompilationFinal private int contextLevel;

private final int numLocals;
private final int maxStackDepth;

private final int frameOnStackMarkerIndex;

public BytecodeLoopNode(final byte[] bytecodes, final int numLocals,
final Object[] literals, final int maxStackDepth,
final int frameOnStackMarkerIndex, final BackJump[] inlinedLoops) {
final int frameOnStackMarkerIndex, final BackJump[] inlinedLoops, int contextLevel) {
this.bytecodesField = bytecodes;
this.numLocals = numLocals;
this.literalsAndConstantsField = literals;
Expand All @@ -172,13 +174,19 @@ public BytecodeLoopNode(final byte[] bytecodes, final int numLocals,
this.frameOnStackMarkerIndex = frameOnStackMarkerIndex;

this.quickenedField = new Node[bytecodes.length];
this.contextLevel = contextLevel;
}

public int getContextLevel() {
return contextLevel;
}

@Override
public Node deepCopy() {
return new BytecodeLoopNode(
bytecodesField.clone(), numLocals, literalsAndConstantsField,
maxStackDepth, frameOnStackMarkerIndex, inlinedLoopsField).initialize(sourceCoord);
maxStackDepth, frameOnStackMarkerIndex, inlinedLoopsField, contextLevel).initialize(
sourceCoord);
}

public String getNameOfLocal(final int idx) {
Expand Down Expand Up @@ -763,7 +771,7 @@ public Object executeGeneric(final VirtualFrame frame) {

Object result = stack[stackPointer];
// stackPointer -= 1;
doReturnNonLocal(frame, bytecodeIndex, result);
doReturnNonLocal(frame, result);
return Nil.nilObject;
}

Expand Down Expand Up @@ -1195,11 +1203,8 @@ private SClass getHolder() {
}

@InliningCutoff
private void doReturnNonLocal(final VirtualFrame frame, final int bytecodeIndex,
final Object result) {
byte contextIdx = bytecodesField[bytecodeIndex + 1];

MaterializedFrame ctx = determineContext(frame, contextIdx);
private void doReturnNonLocal(final VirtualFrame frame, final Object result) {
MaterializedFrame ctx = determineContext(frame, contextLevel);
FrameOnStackMarker marker =
(FrameOnStackMarker) ctx.getObject(frameOnStackMarkerIndex);

Expand Down Expand Up @@ -1265,6 +1270,9 @@ public void replaceAfterScopeChange(final ScopeAdaptationVisitor inliner) {
}
} else {
boolean requiresChangesToContextLevels = inliner.outerScopeChanged();
if (requiresChangesToContextLevels) {
contextLevel -= 1;
}
adapt(inliner, requiresChangesToContextLevels);
}
}
Expand Down Expand Up @@ -1532,12 +1540,10 @@ private void inlineInto(final BytecodeMethodGenContext mgenc,
}

case RETURN_NON_LOCAL: {
byte contextIdx = bytecodes[i + 1];
byte newCtx = (byte) (contextIdx - 1);
if (newCtx == 0) {
emitRETURNLOCAL(mgenc);
} else {
if (mgenc.isBlockMethod()) {
emitRETURNNONLOCAL(mgenc);
} else {
emitRETURNLOCAL(mgenc);
}
break;
}
Expand Down Expand Up @@ -1734,22 +1740,8 @@ private void adapt(final ScopeAdaptationVisitor inliner,
case POP_FIELD_1:
case SEND:
case SUPER_SEND:
case RETURN_LOCAL: {
break;
}

case RETURN_NON_LOCAL: {
byte contextIdx = bytecodes[i + 1];
if (requiresChangesToContextLevels && contextIdx >= inliner.contextLevel) {
// we don't simplify to return local, because they had different bytecode length
// and, well, I don't think this should happen
assert contextIdx - 1 > 0 : "I wouldn't expect a RETURN_LOCAL equivalent here, "
+ " because we are in a block, or it is already a return local";
bytecodes[i + 1] = (byte) (contextIdx - 1);
}
break;
}

case RETURN_LOCAL:
case RETURN_NON_LOCAL:
case RETURN_SELF:
case RETURN_FIELD_0:
case RETURN_FIELD_1:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ public SAbstractObject doSBlock(@SuppressWarnings("unused") final SBlock receive
}
}

@ReportPolymorphism
@GenerateNodeFactory
@GenerateWrapper
@Primitive(className = "Block", primitive = "value")
Expand Down Expand Up @@ -100,7 +99,6 @@ public WrapperNode createWrapper(final ProbeNode probe) {
}
}

@ReportPolymorphism
@GenerateWrapper
@GenerateNodeFactory
@Primitive(className = "Block2", primitive = "value:", selector = "value:", inParser = false,
Expand Down Expand Up @@ -146,7 +144,6 @@ public WrapperNode createWrapper(final ProbeNode probe) {
}
}

@ReportPolymorphism
@GenerateNodeFactory
@Primitive(className = "Block3", primitive = "value:with:", selector = "value:with:",
inParser = false, receiverType = SBlock.class)
Expand Down
6 changes: 3 additions & 3 deletions tests/trufflesom/tests/BytecodeBlockTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,12 @@ private void blockIfReturnNonLocal(final String sel, final byte jumpBytecode) {
+ " #end\n"
+ "]");

assertEquals(17, bytecodes.length);
assertEquals(16, bytecodes.length);
check(bytecodes,
t(5, Bytecodes.SEND),
new BC(jumpBytecode, 6),
new BC(jumpBytecode, 5),
Bytecodes.PUSH_ARG1,
new BC(Bytecodes.RETURN_NON_LOCAL, 1),
Bytecodes.RETURN_NON_LOCAL,
Bytecodes.POP);
}

Expand Down
Loading