Skip to content

Commit

Permalink
Showing 2 changed files with 20 additions and 4 deletions.
3 changes: 0 additions & 3 deletions spec/truffle/tags/core/bignum/right_shift_tags.txt

This file was deleted.

Original file line number Diff line number Diff line change
@@ -453,7 +453,7 @@ public abstract static class RightShiftNode extends BignumCoreMethodNode {
private final BranchProfile bLessThanZero = BranchProfile.create();

@Specialization
public Object leftShift(DynamicObject a, int b) {
public Object rightShift(DynamicObject a, int b) {
if (b >= 0) {
return fixnumOrBignum(Layouts.BIGNUM.getValue(a).shiftRight(b));
} else {
@@ -462,6 +462,25 @@ public Object leftShift(DynamicObject a, int b) {
}
}

@Specialization(guards = "isRubyBignum(b)")
public int rightShift(DynamicObject a, DynamicObject b) {
return 0;
}

@Specialization(guards = {"!isRubyBignum(b)", "!isInteger(b)"})
public Object rightShift(VirtualFrame frame,
DynamicObject a,
Object b,
@Cached("new()") SnippetNode snippetNode) {
int bInt = (int) snippetNode.execute(frame, "Rubinius::Type.coerce_to(y, Integer, :to_int)", "y", b);
if (bInt >= 0) {
return fixnumOrBignum(Layouts.BIGNUM.getValue(a).shiftRight(bInt));
} else {
return fixnumOrBignum(Layouts.BIGNUM.getValue(a).shiftLeft(-bInt));
}

This comment has been minimized.

Copy link
@eregon

eregon Apr 26, 2016

Member

@jruby/truffle It is possible to use a recursive execute here, so if

public abstract Object executeRightShift(VirtualFrame frame, DynamicObject a, Object b);

is added, then one can just call recursively with:
return executeRightShift(frame, a, bInt).
This has the advantage of not repeating guards (and still handle possible extra @Cached arguments).

In this case it might be overkill as calling rightShift(a, bInt) would do the same thing.

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Apr 26, 2016

Contributor

I don't quite follow what you mean?

This comment has been minimized.

Copy link
@eregon

eregon Apr 26, 2016

Member

Here is a self-contained example: bd8c92e

This comment has been minimized.

Copy link
@bjfish

bjfish Apr 26, 2016

Author Contributor

Does calling rightShift versus executeRightShift affect the profile in rightShift differently?

This comment has been minimized.

Copy link
@eregon

eregon Apr 26, 2016

Member

Not in this case since both would use the same node and therefore the same profile.

For additional fixes here (that were there before your commit), the profile should be a ConditionProfile and be stored in the specialization and not in the node,
and in that case using executeRightShift would be much more convenient as one would need to pass a profile to rightShift otherwise.

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Apr 26, 2016

Contributor

Do we think the DSL should provide a mechanism to call an @Specialization with @Cached without having to supply them yourself?

This comment has been minimized.

Copy link
@eregon

eregon Apr 26, 2016

Member

@chrisseaton abstract executeX methods basically allow that.

}


}

@CoreMethod(names = { "abs", "magnitude" })

1 comment on commit 5ade4ed

@bjfish
Copy link
Contributor Author

@bjfish bjfish commented on 5ade4ed Apr 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @eregon , I've refactored this specialization here: 808c0a8

Please sign in to comment.