[GR-19220] Inline AddNode specialization that are not yet inlined int…

…o InlinedAddNode (#2518) PullRequest: truffleruby/3022
oracle · Oct 29, 2021 · f2d856f · f2d856f
2 parents 2e3e901 + 86b9427
commit f2d856f
Show file tree

Hide file tree

Showing 10 changed files with 74 additions and 49 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,6 +51,7 @@ Performance:
 * `String#sub`, `sub!`, `gsub`, and `gsub!` have been refactored for better performance.
 * Don't allocate a `MatchData` object when `Regexp#match?` or `String#match?` is used (#2509, @nirvdrum).
 * Add `ENV.except` (#2507, @Strech).
+* Fully inline the `Integer#+` and `Integer#-` logic for interpreter speed (#2518, @smarr).
 
 Changes:
 

diff --git a/spec/ruby/core/integer/plus_spec.rb b/spec/ruby/core/integer/plus_spec.rb
@@ -40,4 +40,19 @@
       -> { @bignum + :symbol}.should raise_error(TypeError)
     end
   end
+
+  it "can be redefined" do
+    code = <<~RUBY
+      class Integer
+        alias_method :old_plus, :+
+        def +(other)
+          self - other
+        end
+      end
+      result = 1 + 2
+      Integer.alias_method :+, :old_plus
+      print result
+    RUBY
+    ruby_exe(code).should == "-1"
+  end
 end
diff --git a/spec/tags/core/integer/plus_tags.txt b/spec/tags/core/integer/plus_tags.txt
@@ -0,0 +1 @@
+slow:Integer#+ can be redefined
diff --git a/src/main/java/org/truffleruby/core/inlined/InlinedAddNode.java b/src/main/java/org/truffleruby/core/inlined/InlinedAddNode.java
@@ -10,18 +10,18 @@
 package org.truffleruby.core.inlined;
 
 import org.truffleruby.RubyLanguage;
-import org.truffleruby.core.numeric.IntegerNodes.AddNode;
-import org.truffleruby.core.numeric.IntegerNodesFactory.AddNodeFactory;
+import org.truffleruby.core.numeric.BigIntegerOps;
+import org.truffleruby.core.numeric.FixnumOrBignumNode;
 import org.truffleruby.language.dispatch.RubyCallNodeParameters;
 
-import com.oracle.truffle.api.CompilerDirectives;
+import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.VirtualFrame;
 
+/** See {@link org.truffleruby.core.numeric.IntegerNodes.AddNode} and
+ * {@link org.truffleruby.core.numeric.FloatNodes.AddNode} */
 public abstract class InlinedAddNode extends BinaryInlinedOperationNode {
 
-    @Child AddNode fixnumAdd;
-
     public InlinedAddNode(RubyLanguage language, RubyCallNodeParameters callNodeParameters) {
         super(
                 language,
@@ -30,14 +30,25 @@ public InlinedAddNode(RubyLanguage language, RubyCallNodeParameters callNodePara
                 language.coreMethodAssumptions.floatAddAssumption);
     }
 
+    @Specialization(rewriteOn = ArithmeticException.class, assumptions = "assumptions")
+    protected int intAdd(int a, int b) {
+        return Math.addExact(a, b);
+    }
+
     @Specialization(assumptions = "assumptions")
-    protected Object intAdd(int a, int b) {
-        return getAddNode().executeAdd(a, b);
+    protected long intAddWithOverflow(int a, int b) {
+        return (long) a + (long) b;
+    }
+
+    @Specialization(rewriteOn = ArithmeticException.class, assumptions = "assumptions")
+    protected long longAdd(long a, long b) {
+        return Math.addExact(a, b);
     }
 
     @Specialization(assumptions = "assumptions")
-    protected Object longAdd(long a, long b) {
-        return getAddNode().executeAdd(a, b);
+    protected Object longAddWithOverflow(long a, long b,
+            @Cached FixnumOrBignumNode fixnumOrBignum) {
+        return fixnumOrBignum.fixnumOrBignum(BigIntegerOps.add(a, b));
     }
 
     @Specialization(assumptions = "assumptions")
@@ -59,13 +70,4 @@ protected double doubleLong(double a, long b) {
     protected Object fallback(VirtualFrame frame, Object a, Object b) {
         return rewriteAndCall(frame, a, b);
     }
-
-    private AddNode getAddNode() {
-        if (fixnumAdd == null) {
-            CompilerDirectives.transferToInterpreterAndInvalidate();
-            fixnumAdd = insert(AddNodeFactory.create(null));
-        }
-        return fixnumAdd;
-    }
-
 }
diff --git a/src/main/java/org/truffleruby/core/inlined/InlinedSubNode.java b/src/main/java/org/truffleruby/core/inlined/InlinedSubNode.java
@@ -9,19 +9,19 @@
  */
 package org.truffleruby.core.inlined;
 
+import com.oracle.truffle.api.dsl.Cached;
 import org.truffleruby.RubyLanguage;
-import org.truffleruby.core.numeric.IntegerNodes.SubNode;
-import org.truffleruby.core.numeric.IntegerNodesFactory.SubNodeFactory;
+import org.truffleruby.core.numeric.BigIntegerOps;
+import org.truffleruby.core.numeric.FixnumOrBignumNode;
 import org.truffleruby.language.dispatch.RubyCallNodeParameters;
 
-import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.VirtualFrame;
 
+/** See {@link org.truffleruby.core.numeric.IntegerNodes.SubNode} and
+ * {@link org.truffleruby.core.numeric.FloatNodes.SubNode} */
 public abstract class InlinedSubNode extends BinaryInlinedOperationNode {
 
-    @Child SubNode fixnumSub;
-
     public InlinedSubNode(RubyLanguage language, RubyCallNodeParameters callNodeParameters) {
         super(
                 language,
@@ -30,14 +30,25 @@ public InlinedSubNode(RubyLanguage language, RubyCallNodeParameters callNodePara
                 language.coreMethodAssumptions.floatSubAssumption);
     }
 
+    @Specialization(rewriteOn = ArithmeticException.class, assumptions = "assumptions")
+    protected int intSub(int a, int b) {
+        return Math.subtractExact(a, b);
+    }
+
     @Specialization(assumptions = "assumptions")
-    protected Object intSub(int a, int b) {
-        return getSubNode().executeSub(a, b);
+    protected long intSubWithOverflow(int a, int b) {
+        return (long) a - (long) b;
+    }
+
+    @Specialization(rewriteOn = ArithmeticException.class, assumptions = "assumptions")
+    protected long longSub(long a, long b) {
+        return Math.subtractExact(a, b);
     }
 
     @Specialization(assumptions = "assumptions")
-    protected Object longSub(long a, long b) {
-        return getSubNode().executeSub(a, b);
+    protected Object longSubWithOverflow(long a, long b,
+            @Cached FixnumOrBignumNode fixnumOrBignumNode) {
+        return fixnumOrBignumNode.fixnumOrBignum(BigIntegerOps.subtract(a, b));
     }
 
     @Specialization(assumptions = "assumptions")
@@ -59,13 +70,4 @@ protected double doubleLong(double a, long b) {
     protected Object fallback(VirtualFrame frame, Object a, Object b) {
         return rewriteAndCall(frame, a, b);
     }
-
-    private SubNode getSubNode() {
-        if (fixnumSub == null) {
-            CompilerDirectives.transferToInterpreterAndInvalidate();
-            fixnumSub = insert(SubNodeFactory.create(null));
-        }
-        return fixnumSub;
-    }
-
 }
diff --git a/src/main/java/org/truffleruby/core/numeric/FixnumOrBignumNode.java b/src/main/java/org/truffleruby/core/numeric/FixnumOrBignumNode.java
@@ -23,6 +23,10 @@ public class FixnumOrBignumNode extends RubyBaseNode {
     private static final BigInteger LONG_MIN_BIGINT = BigInteger.valueOf(Long.MIN_VALUE);
     private static final BigInteger LONG_MAX_BIGINT = BigInteger.valueOf(Long.MAX_VALUE);
 
+    public static FixnumOrBignumNode create() {
+        return new FixnumOrBignumNode();
+    }
+
     public FixnumOrBignumNode() {
     }
 

diff --git a/src/main/java/org/truffleruby/core/numeric/FloatNodes.java b/src/main/java/org/truffleruby/core/numeric/FloatNodes.java
@@ -52,6 +52,7 @@ protected double neg(double value) {
 
     }
 
+    /** See {@link org.truffleruby.core.inlined.InlinedAddNode} */
     @CoreMethod(names = "+", required = 1)
     public abstract static class AddNode extends CoreMethodArrayArgumentsNode {
 
@@ -77,6 +78,7 @@ protected Object addCoerced(double a, Object b,
         }
     }
 
+    /** See {@link org.truffleruby.core.inlined.InlinedSubNode} */
     @CoreMethod(names = "-", required = 1)
     public abstract static class SubNode extends CoreMethodArrayArgumentsNode {
 
@@ -639,7 +641,7 @@ protected long roundFittingLongNegative(double n) {
 
         @Specialization(guards = "isPositive(n)", replaces = "roundFittingLongPositive")
         protected Object roundPositive(double n,
-                @Cached("new()") FixnumOrBignumNode fixnumOrBignum) {
+                @Cached FixnumOrBignumNode fixnumOrBignum) {
             double f = Math.floor(n);
             if (n - f >= 0.5) {
                 f += 1.0;
@@ -649,7 +651,7 @@ protected Object roundPositive(double n,
 
         @Specialization(guards = "!isPositive(n)", replaces = "roundFittingLongNegative")
         protected Object roundNegative(double n,
-                @Cached("new()") FixnumOrBignumNode fixnumOrBignum) {
+                @Cached FixnumOrBignumNode fixnumOrBignum) {
             double f = Math.ceil(n);
             if (f - n >= 0.5) {
                 f -= 1.0;
@@ -702,7 +704,7 @@ protected long roundFittingLongNegative(double n) {
 
         @Specialization(guards = "isPositive(n)", replaces = "roundFittingLongPositive")
         protected Object roundPositive(double n,
-                @Cached("new()") FixnumOrBignumNode fixnumOrBignum) {
+                @Cached FixnumOrBignumNode fixnumOrBignum) {
             double f = Math.floor(n);
             if (n - f == 0.5) {
                 f += f % 2;
@@ -712,7 +714,7 @@ protected Object roundPositive(double n,
 
         @Specialization(guards = "!isPositive(n)", replaces = "roundFittingLongNegative")
         protected Object roundNegative(double n,
-                @Cached("new()") FixnumOrBignumNode fixnumOrBignum) {
+                @Cached FixnumOrBignumNode fixnumOrBignum) {
             double f = Math.ceil(n);
             if (n - f == 0.5) {
                 f -= f % 2;
@@ -764,7 +766,7 @@ protected long roundFittingLongNegative(double n) {
 
         @Specialization(guards = "isPositive(n)", replaces = "roundFittingLongPositive")
         protected Object roundPositive(double n,
-                @Cached("new()") FixnumOrBignumNode fixnumOrBignum) {
+                @Cached FixnumOrBignumNode fixnumOrBignum) {
             double f = Math.floor(n);
             if (n - f > 0.5) {
                 f += 1.0;
@@ -774,7 +776,7 @@ protected Object roundPositive(double n,
 
         @Specialization(guards = "!isPositive(n)", replaces = "roundFittingLongNegative")
         protected Object roundNegative(double n,
-                @Cached("new()") FixnumOrBignumNode fixnumOrBignum) {
+                @Cached FixnumOrBignumNode fixnumOrBignum) {
             double f = Math.ceil(n);
             if (f - n > 0.5) {
                 f -= 1.0;

diff --git a/src/main/java/org/truffleruby/core/numeric/IntegerNodes.java b/src/main/java/org/truffleruby/core/numeric/IntegerNodes.java
@@ -111,11 +111,10 @@ protected Object doObject(RubyBignum value) {
 
     }
 
+    /** See {@link org.truffleruby.core.inlined.InlinedAddNode} */
     @CoreMethod(names = "+", required = 1)
     public abstract static class AddNode extends BignumCoreMethodNode {
 
-        public abstract Object executeAdd(Object a, Object b);
-
         @Specialization(rewriteOn = ArithmeticException.class)
         protected int add(int a, int b) {
             return Math.addExact(a, b);
@@ -168,11 +167,10 @@ protected Object addCoerced(Object a, Object b,
         }
     }
 
+    /** See {@link org.truffleruby.core.inlined.InlinedSubNode} */
     @CoreMethod(names = "-", required = 1)
     public abstract static class SubNode extends BignumCoreMethodNode {
 
-        public abstract Object executeSub(Object a, Object b);
-
         @Specialization(rewriteOn = ArithmeticException.class)
         protected int sub(int a, int b) {
             return Math.subtractExact(a, b);

diff --git a/src/main/java/org/truffleruby/core/string/StringNodes.java b/src/main/java/org/truffleruby/core/string/StringNodes.java
@@ -4180,7 +4180,7 @@ public abstract static class StringToFPrimitiveNode extends PrimitiveArrayArgume
         @Specialization
         protected Object stringToF(Object string,
                 @CachedLibrary(limit = "2") RubyStringLibrary strings,
-                @Cached("new()") FixnumOrBignumNode fixnumOrBignumNode,
+                @Cached FixnumOrBignumNode fixnumOrBignumNode,
                 @Cached BytesNode bytesNode) {
             final Rope rope = strings.getRope(string);
             if (rope.isEmpty()) {
@@ -5288,7 +5288,7 @@ protected int stringToInumIntRope(Rope rope, int fixBase, boolean strict, boolea
 
         @Specialization(guards = "!isLazyIntRopeOptimizable(rope, fixBase)")
         protected Object stringToInum(Rope rope, int fixBase, boolean strict, boolean raiseOnError,
-                @Cached("new()") FixnumOrBignumNode fixnumOrBignumNode,
+                @Cached FixnumOrBignumNode fixnumOrBignumNode,
                 @Cached BytesNode bytesNode,
                 @Cached BranchProfile exceptionProfile) {
             try {

diff --git a/src/main/java/org/truffleruby/core/support/RandomizerNodes.java b/src/main/java/org/truffleruby/core/support/RandomizerNodes.java
@@ -82,7 +82,7 @@ protected long randomizerRandInt(RubyRandomizer randomizer, long limit) {
 
         @Specialization
         protected Object randomizerRandInt(RubyRandomizer randomizer, RubyBignum limit,
-                @Cached("new()") FixnumOrBignumNode fixnumOrBignum) {
+                @Cached FixnumOrBignumNode fixnumOrBignum) {
             return fixnumOrBignum.fixnumOrBignum(randLimitedBignum(randomizer, limit.value));
         }