Skip to content

Commit

Permalink
[Truffle] Break down IfNode into If, IfElse and Unless so they can be…
Browse files Browse the repository at this point in the history 
… simpler.
@chrisseaton
chrisseaton committed Feb 17, 2016
1 parent 2f55695 commit 88da599
Showing 6 changed files with 120 additions and 29 deletions.
45 changes: 45 additions & 0 deletions truffle/src/main/java/org/jruby/truffle/language/control/IfElseNode.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright (c) 2013, 2016 Oracle and/or its affiliates. All rights reserved. This
* code is released under a tri EPL/GPL/LGPL license. You can use it,
* redistribute it and/or modify it under the terms of the:
*
* Eclipse Public License version 1.0
* GNU General Public License version 2
* GNU Lesser General Public License version 2.1
*/
package org.jruby.truffle.language.control;

import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.api.source.SourceSection;
import org.jruby.truffle.RubyContext;
import org.jruby.truffle.core.cast.BooleanCastNode;
import org.jruby.truffle.core.cast.BooleanCastNodeGen;
import org.jruby.truffle.language.RubyNode;

public class IfElseNode extends RubyNode {

@Child private BooleanCastNode condition;
@Child private RubyNode thenBody;
@Child private RubyNode elseBody;

private final ConditionProfile conditionProfile = ConditionProfile.createCountingProfile();

public IfElseNode(RubyContext context, SourceSection sourceSection, RubyNode condition, RubyNode thenBody, RubyNode elseBody) {
super(context, sourceSection);

this.condition = BooleanCastNodeGen.create(context, sourceSection, condition);
this.thenBody = thenBody;
this.elseBody = elseBody;
}

@Override
public Object execute(VirtualFrame frame) {
if (conditionProfile.profile(condition.executeBoolean(frame))) {
return thenBody.execute(frame);
} else {
return elseBody.execute(frame);
}
}

}
6 changes: 2 additions & 4 deletions truffle/src/main/java/org/jruby/truffle/language/control/IfNode.java
View file
Original file line number Diff line number Diff line change
@@ -21,24 +21,22 @@ public class IfNode extends RubyNode {

@Child private BooleanCastNode condition;
@Child private RubyNode thenBody;
@Child private RubyNode elseBody;

private final ConditionProfile conditionProfile = ConditionProfile.createCountingProfile();

public IfNode(RubyContext context, SourceSection sourceSection, RubyNode condition, RubyNode thenBody, RubyNode elseBody) {
public IfNode(RubyContext context, SourceSection sourceSection, RubyNode condition, RubyNode thenBody) {
super(context, sourceSection);

this.condition = BooleanCastNodeGen.create(context, sourceSection, condition);
this.thenBody = thenBody;
this.elseBody = elseBody;
}

@Override
public Object execute(VirtualFrame frame) {
if (conditionProfile.profile(condition.executeBoolean(frame))) {
return thenBody.execute(frame);
} else {
return elseBody.execute(frame);
return nil();
}
}

43 changes: 43 additions & 0 deletions truffle/src/main/java/org/jruby/truffle/language/control/UnlessNode.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright (c) 2013, 2016 Oracle and/or its affiliates. All rights reserved. This
* code is released under a tri EPL/GPL/LGPL license. You can use it,
* redistribute it and/or modify it under the terms of the:
*
* Eclipse Public License version 1.0
* GNU General Public License version 2
* GNU Lesser General Public License version 2.1
*/
package org.jruby.truffle.language.control;

import com.oracle.truffle.api.frame.VirtualFrame;
import com.oracle.truffle.api.profiles.ConditionProfile;
import com.oracle.truffle.api.source.SourceSection;
import org.jruby.truffle.RubyContext;
import org.jruby.truffle.core.cast.BooleanCastNode;
import org.jruby.truffle.core.cast.BooleanCastNodeGen;
import org.jruby.truffle.language.RubyNode;

public class UnlessNode extends RubyNode {

@Child private BooleanCastNode condition;
@Child private RubyNode thenBody;

private final ConditionProfile conditionProfile = ConditionProfile.createCountingProfile();

public UnlessNode(RubyContext context, SourceSection sourceSection, RubyNode condition, RubyNode thenBody) {
super(context, sourceSection);

this.condition = BooleanCastNodeGen.create(context, sourceSection, condition);
this.thenBody = thenBody;
}

@Override
public Object execute(VirtualFrame frame) {
if (conditionProfile.profile(!condition.executeBoolean(frame))) {

This comment has been minimized.

Sign in to view

Copy link
@eregon

eregon Feb 24, 2016

Member

I think it would be more intuitive to have the ! before the profile. Also the profile value would match with the condition value.

This comment has been minimized.

Sign in to view

Copy link
@chrisseaton

chrisseaton Feb 24, 2016

Author Contributor

Done.
return thenBody.execute(frame);
} else {
return nil();
}
}

}
39 changes: 22 additions & 17 deletions truffle/src/main/java/org/jruby/truffle/language/translator/BodyTranslator.java
View file
Original file line number Diff line number Diff line change
@@ -85,6 +85,7 @@
import org.jruby.truffle.language.control.BreakNode;
import org.jruby.truffle.language.control.ElidableResultNode;
import org.jruby.truffle.language.control.FrameOnStackNode;
import org.jruby.truffle.language.control.IfElseNode;
import org.jruby.truffle.language.control.IfNode;
import org.jruby.truffle.language.control.NextNode;
import org.jruby.truffle.language.control.NotNode;
@@ -96,6 +97,7 @@
import org.jruby.truffle.language.control.ReturnID;
import org.jruby.truffle.language.control.ReturnNode;
import org.jruby.truffle.language.control.SequenceNode;
import org.jruby.truffle.language.control.UnlessNode;
import org.jruby.truffle.language.control.WhenSplatNode;
import org.jruby.truffle.language.control.WhileNode;
import org.jruby.truffle.language.defined.DefinedNode;
@@ -864,7 +866,7 @@ public RubyNode visitCaseNode(org.jruby.ast.CaseNode node) {

final RubyNode thenNode = translateNodeOrNil(sourceSection, when.getBodyNode());

final IfNode ifNode = new IfNode(context, sourceSection, conditionNode, thenNode, elseNode);
final IfElseNode ifNode = new IfElseNode(context, sourceSection, conditionNode, thenNode, elseNode);

// This if becomes the else for the next if

@@ -909,7 +911,7 @@ public RubyNode visitCaseNode(org.jruby.ast.CaseNode node) {

final RubyNode thenNode = when.getBodyNode().accept(this);

final IfNode ifNode = new IfNode(context, sourceSection, conditionNode, thenNode, elseNode);
final IfElseNode ifNode = new IfElseNode(context, sourceSection, conditionNode, thenNode, elseNode);

// This if becomes the else for the next if

@@ -1744,24 +1746,27 @@ public RubyNode visitHashNode(org.jruby.ast.HashNode node) {
public RubyNode visitIfNode(org.jruby.ast.IfNode node) {
final SourceSection sourceSection = translate(node.getPosition());

org.jruby.ast.Node thenBody = node.getThenBody();

if (thenBody == null || thenBody.isNil()) {
thenBody = new org.jruby.ast.NilNode(node.getPosition());
}
final RubyNode condition = translateNodeOrNil(sourceSection, node.getCondition());

org.jruby.ast.Node thenBody = node.getThenBody();
org.jruby.ast.Node elseBody = node.getElseBody();

if (elseBody == null || elseBody.isNil()) {
elseBody = new org.jruby.ast.NilNode(node.getPosition());
}

final RubyNode condition = translateNodeOrNil(sourceSection, node.getCondition());
final RubyNode ret;

final RubyNode thenBodyTranslated = thenBody.accept(this);
final RubyNode elseBodyTranslated = elseBody.accept(this);
if (thenBody != null && elseBody != null) {
final RubyNode thenBodyTranslated = thenBody.accept(this);
final RubyNode elseBodyTranslated = elseBody.accept(this);
ret = new IfElseNode(context, sourceSection, condition, thenBodyTranslated, elseBodyTranslated);
} else if (thenBody != null) {
final RubyNode thenBodyTranslated = thenBody.accept(this);
ret = new IfNode(context, sourceSection, condition, thenBodyTranslated);
} else if (elseBody != null) {
final RubyNode elseBodyTranslated = elseBody.accept(this);
ret = new UnlessNode(context, sourceSection, condition, elseBodyTranslated);
} else {
ret = SequenceNode.sequence(context, sourceSection, condition, new NilNode(context, sourceSection));
}

final RubyNode ret = new IfNode(context, sourceSection, condition, thenBodyTranslated, elseBodyTranslated);
return addNewlineIfNeeded(node, ret);
}

@@ -2277,7 +2282,7 @@ public RubyNode visitMultipleAsgnNode(org.jruby.ast.MultipleAsgnNode node) {
final RubyNode atLeastAsLarge = SequenceNode.sequence(context, sourceSection, atLeastAsLargeSequence);

final RubyNode assignPost =
new IfNode(context, sourceSection,
new IfElseNode(context, sourceSection,
new ArrayIsAtLeastAsLargeAsNode(context, sourceSection, environment.findLocalVarNode(tempName, sourceSection), node.getPreCount() + node.getPostCount()),
atLeastAsLarge,
smaller);
@@ -2417,7 +2422,7 @@ public RubyNode visitMultipleAsgnNode(org.jruby.ast.MultipleAsgnNode node) {
final RubyNode atLeastAsLarge = SequenceNode.sequence(context, sourceSection, atLeastAsLargeSequence);

final RubyNode assignPost =
new IfNode(context, sourceSection,
new IfElseNode(context, sourceSection,
new ArrayIsAtLeastAsLargeAsNode(context, sourceSection, environment.findLocalVarNode(tempName, sourceSection), node.getPreCount() + node.getPostCount()),
atLeastAsLarge,
smaller);
12 changes: 6 additions & 6 deletions truffle/src/main/java/org/jruby/truffle/language/translator/LoadArgumentsTranslator.java
View file
Original file line number Diff line number Diff line change
@@ -37,6 +37,7 @@
import org.jruby.truffle.language.arguments.ReadPreArgumentNode;
import org.jruby.truffle.language.arguments.ReadRestArgumentNode;
import org.jruby.truffle.language.arguments.RunBlockKWArgsHelperNode;
import org.jruby.truffle.language.control.IfElseNode;
import org.jruby.truffle.language.control.IfNode;
import org.jruby.truffle.language.control.SequenceNode;
import org.jruby.truffle.language.literal.NilNode;
@@ -122,8 +123,7 @@ public RubyNode visitArgsNode(org.jruby.ast.ArgsNode node) {

sequence.add(new IfNode(context, sourceSection,
new ArrayIsAtLeastAsLargeAsNode(context, sourceSection, loadArray(sourceSection), node.getPreCount() + node.getPostCount()),
new RunBlockKWArgsHelperNode(context, sourceSection, arraySlotStack.peek().getArraySlot(), keyRestNameOrNil),
new NilNode(context, sourceSection)));
new RunBlockKWArgsHelperNode(context, sourceSection, arraySlotStack.peek().getArraySlot(), keyRestNameOrNil)));
}

final int preCount = node.getPreCount();
@@ -216,7 +216,7 @@ public RubyNode visitArgsNode(org.jruby.ast.ArgsNode node) {

if (useArray()) {
if (node.getPreCount() == 0 || node.hasRestArg()) {
sequence.add(new IfNode(context, sourceSection,
sequence.add(new IfElseNode(context, sourceSection,
new ArrayIsAtLeastAsLargeAsNode(context, sourceSection, loadArray(sourceSection), node.getPreCount() + node.getPostCount()),
notNilAtLeastAsLarge,
notNilSmaller));
@@ -410,7 +410,7 @@ private RubyNode translateLocalAssignment(ISourcePosition sourcePosition, String

if (useArray()) {
// TODO CS 10-Jan-16 we should really hoist this check, or see if Graal does it for us
readNode = new IfNode(context, sourceSection,
readNode = new IfElseNode(context, sourceSection,
new ArrayIsAtLeastAsLargeAsNode(context, sourceSection, loadArray(sourceSection), minimum),
PrimitiveArrayNodeFactory.read(context, sourceSection, loadArray(sourceSection), index),
defaultValue);
@@ -585,10 +585,10 @@ public RubyNode visitMultipleAsgnNode(MultipleAsgnNode node) {
new WriteLocalVariableNode(context, sourceSection,
SplatCastNodeGen.create(context, sourceSection, SplatCastNode.NilBehavior.ARRAY_WITH_NIL, true,
readArgument(sourceSection)), arraySlot),
new IfNode(context, sourceSection,
new IfElseNode(context, sourceSection,
new IsNilNode(context, sourceSection, new ReadLocalVariableNode(context, sourceSection, arraySlot)),
nil,
new IfNode(context, sourceSection,
new IfElseNode(context, sourceSection,
new ArrayIsAtLeastAsLargeAsNode(context, sourceSection, new ReadLocalVariableNode(context, sourceSection, arraySlot), node.getPreCount() + node.getPostCount()),
notNilAtLeastAsLarge,
notNilSmaller)));
4 changes: 2 additions & 2 deletions truffle/src/main/java/org/jruby/truffle/language/translator/MethodTranslator.java
View file
Original file line number Diff line number Diff line change
@@ -36,7 +36,7 @@
import org.jruby.truffle.language.arguments.ReadPreArgumentNode;
import org.jruby.truffle.language.arguments.ShouldDestructureNode;
import org.jruby.truffle.language.control.AndNode;
import org.jruby.truffle.language.control.IfNode;
import org.jruby.truffle.language.control.IfElseNode;
import org.jruby.truffle.language.control.NotNode;
import org.jruby.truffle.language.control.SequenceNode;
import org.jruby.truffle.language.dispatch.RespondToNode;
@@ -114,7 +114,7 @@ public BlockDefinitionNode compileBlockNode(SourceSection sourceSection, String
shouldDestructure,
arrayWasNotNil);

preludeProc = new IfNode(context, sourceSection,
preludeProc = new IfElseNode(context, sourceSection,
shouldDestructureAndArrayWasNotNil,
newDestructureArguments,
loadArguments);

0 comments on commit 88da599

Please sign in to comment.