Skip to content

Commit

Permalink
[Truffle] Tidy up ReadDeclarationVariableNode.
Browse files Browse the repository at this point in the history
@chrisseaton
chrisseaton committed Feb 23, 2016
1 parent a22364a commit 870a9e9
Showing 3 changed files with 53 additions and 34 deletions.
5 changes: 3 additions & 2 deletions truffle/src/main/java/org/jruby/truffle/core/array/ArrayNodes.java
View file
Original file line number Diff line number Diff line change
@@ -76,6 +76,7 @@
import org.jruby.truffle.language.dispatch.CallDispatchHeadNode;
import org.jruby.truffle.language.dispatch.DispatchHeadNodeFactory;
import org.jruby.truffle.language.dispatch.MissingBehavior;
import org.jruby.truffle.language.locals.LocalVariableType;
import org.jruby.truffle.language.locals.ReadDeclarationVariableNode;
import org.jruby.truffle.language.methods.Arity;
import org.jruby.truffle.language.methods.DeclarationContext;
@@ -2243,7 +2244,7 @@ public MaxBlock(RubyContext context) {
sharedMethodInfo = new SharedMethodInfo(sourceSection, null, Arity.NO_ARGUMENTS, "max", false, null, false, false, false);

callTarget = Truffle.getRuntime().createCallTarget(new RubyRootNode(context, sourceSection, null, sharedMethodInfo, ArrayNodesFactory.MaxBlockNodeFactory.create(context, sourceSection, new RubyNode[]{
new ReadDeclarationVariableNode(context, sourceSection, 1, frameSlot),
new ReadDeclarationVariableNode(context, sourceSection, LocalVariableType.FRAME_LOCAL, 1, frameSlot),
new ReadPreArgumentNode(context, sourceSection, 0, MissingArgumentBehaviour.RUNTIME_ERROR)
}), false));
}
@@ -2361,7 +2362,7 @@ public MinBlock(RubyContext context) {
sharedMethodInfo = new SharedMethodInfo(sourceSection, null, Arity.NO_ARGUMENTS, "min", false, null, false, false, false);

callTarget = Truffle.getRuntime().createCallTarget(new RubyRootNode(context, sourceSection, null, sharedMethodInfo, ArrayNodesFactory.MinBlockNodeFactory.create(context, sourceSection, new RubyNode[]{
new ReadDeclarationVariableNode(context, sourceSection, 1, frameSlot),
new ReadDeclarationVariableNode(context, sourceSection, LocalVariableType.FRAME_LOCAL, 1, frameSlot),
new ReadPreArgumentNode(context, sourceSection, 0, MissingArgumentBehaviour.RUNTIME_ERROR)
}), false));
}
60 changes: 39 additions & 21 deletions truffle/src/main/java/org/jruby/truffle/language/locals/ReadDeclarationVariableNode.java
View file
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@

import com.oracle.truffle.api.CompilerDirectives;
import com.oracle.truffle.api.frame.FrameSlot;
import com.oracle.truffle.api.frame.MaterializedFrame;
import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.source.SourceSection;
import org.jcodings.specific.UTF8Encoding;
@@ -26,44 +27,61 @@

public class ReadDeclarationVariableNode extends ReadLocalNode {

private final LocalVariableType type;
private final int frameDepth;
private final FrameSlot frameSlot;

@Child private ReadFrameSlotNode readFrameSlotNode;

public ReadDeclarationVariableNode(RubyContext context, SourceSection sourceSection,
int frameDepth, FrameSlot slot) {
public ReadDeclarationVariableNode(RubyContext context, SourceSection sourceSection, LocalVariableType type,
int frameDepth, FrameSlot frameSlot) {
super(context, sourceSection);
readFrameSlotNode = ReadFrameSlotNodeGen.create(slot);
this.type = type;
this.frameDepth = frameDepth;
this.frameSlot = frameSlot;
}

@Override
public Object execute(VirtualFrame frame) {
return readFrameSlotNode.executeRead(RubyArguments.getDeclarationFrame(frame, frameDepth));
checkReadFrameSlotNode();
final MaterializedFrame declarationFrame = RubyArguments.getDeclarationFrame(frame, frameDepth);
return readFrameSlotNode.executeRead(declarationFrame);
}


@Override
public RubyNode makeWriteNode(RubyNode rhs) {
return new WriteDeclarationVariableNode(getContext(), getSourceSection(),
rhs, frameDepth, readFrameSlotNode.getFrameSlot());
public Object isDefined(VirtualFrame frame) {
switch (type) {
case FRAME_LOCAL:
return coreStrings().LOCAL_VARIABLE.createInstance();

case FRAME_LOCAL_GLOBAL:
checkReadFrameSlotNode();

if (readFrameSlotNode.executeRead(frame) != nil()) {
return coreStrings().GLOBAL_VARIABLE.createInstance();
} else {
return nil();
}

case ALWAYS_DEFINED_GLOBAL:
return coreStrings().GLOBAL_VARIABLE.createInstance();

default:
throw new UnsupportedOperationException("didn't expect local type " + type);
}
}

public static final Set<String> ALWAYS_DEFINED_GLOBALS = new HashSet<>(Collections.singletonList("$~"));
private void checkReadFrameSlotNode() {
if (readFrameSlotNode == null) {
CompilerDirectives.transferToInterpreter();
readFrameSlotNode = insert(ReadFrameSlotNodeGen.create(frameSlot));
}

This comment has been minimized.

Sign in to view

Copy link
@eregon

eregon Feb 24, 2016

Member

I think we should use the pattern getReadFrameSlotNode() so side-effects are hidden in the specialization.

This comment has been minimized.

Sign in to view

Copy link
@eregon

eregon Feb 24, 2016

Member

readFrameSlot(Frame frame) as in https://github.com/jruby/jruby/wiki/Developing-with-JRuby-Truffle#the-lazy-pattern would be even better and more consistent with our guidelines actually.

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Feb 24, 2016

Author Contributor

Done. I'll go back and look at other nodes' lazy allocation that I've refactored as well.
}

@Override
public Object isDefined(VirtualFrame frame) {
CompilerDirectives.transferToInterpreter();

if (Translator.FRAME_LOCAL_GLOBAL_VARIABLES.contains(readFrameSlotNode.getFrameSlot().getIdentifier())) {
if (ALWAYS_DEFINED_GLOBALS.contains(readFrameSlotNode.getFrameSlot().getIdentifier())
|| readFrameSlotNode.executeRead(RubyArguments.getDeclarationFrame(frame, frameDepth)) != nil()) {
return create7BitString("global-variable", UTF8Encoding.INSTANCE);
} else {
return nil();
}
} else {
return create7BitString("local-variable", UTF8Encoding.INSTANCE);
}
public RubyNode makeWriteNode(RubyNode rhs) {
return new WriteDeclarationVariableNode(getContext(), getSourceSection(), rhs, frameDepth, frameSlot);
}

}
22 changes: 11 additions & 11 deletions truffle/src/main/java/org/jruby/truffle/language/translator/TranslatorEnvironment.java
View file
Original file line number Diff line number Diff line change
@@ -147,22 +147,22 @@ public ReadLocalNode findLocalVarNode(String name, SourceSection sourceSection)
level++;
FrameSlot slot = current.getFrameDescriptor().findFrameSlot(name);
if (slot != null) {
if (level == 0) {
final LocalVariableType type;

if (Translator.FRAME_LOCAL_GLOBAL_VARIABLES.contains(name)) {
if (Translator.ALWAYS_DEFINED_GLOBALS.contains(name)) {
type = LocalVariableType.ALWAYS_DEFINED_GLOBAL;
} else {
type = LocalVariableType.FRAME_LOCAL_GLOBAL;
}
final LocalVariableType type;

if (Translator.FRAME_LOCAL_GLOBAL_VARIABLES.contains(name)) {
if (Translator.ALWAYS_DEFINED_GLOBALS.contains(name)) {
type = LocalVariableType.ALWAYS_DEFINED_GLOBAL;
} else {
type = LocalVariableType.FRAME_LOCAL;
type = LocalVariableType.FRAME_LOCAL_GLOBAL;
}
} else {
type = LocalVariableType.FRAME_LOCAL;
}

if (level == 0) {
return new ReadLocalVariableNode(context, sourceSection, type, slot);
} else {
return new ReadDeclarationVariableNode(context, sourceSection, level, slot);
return new ReadDeclarationVariableNode(context, sourceSection, type, level, slot);
}
}

0 comments on commit 870a9e9

Please sign in to comment.