Add inlining for #to:do: loops (#36)

This is based on SOM-st/PySOM#35 which was never
merged because of the impact on the JIT compiler.
Though, since we have a pure interpreter here, it's a good win on
benchmarks that use a `#to:do:` loop.

The median run time is reduced by only 2% though.


https://rebench.dev/SOMpp/compare/692cc6c47727a7fa76923daf92e0bb8c7d4be3b1..252923132894fff18b7d1364dfb2981f439f146c

Author: smarr

src/compiler/BytecodeGenerator.cpp

@@ -254,6 +254,14 @@ void EmitRETURNNONLOCAL(MethodGenerationContext& mgenc) {
     Emit1(mgenc, BC_RETURN_NON_LOCAL, 0);
 }
 
+void EmitINC(MethodGenerationContext& mgenc) {
+    Emit1(mgenc, BC_INC, 0);
+}
+
+void EmitDupSecond(MethodGenerationContext& mgenc) {
+    Emit1(mgenc, BC_DUP_SECOND, 1);
+}
+
 size_t EmitJumpOnBoolWithDummyOffset(MethodGenerationContext& mgenc,
                                      bool isIfTrue, bool needsPop) {
     // Remember: true and false seem flipped here.
@@ -285,6 +293,13 @@ size_t EmitJumpWithDumyOffset(MethodGenerationContext& mgenc) {
     return idx;
 }
 
+size_t EmitJumpIfGreaterWithDummyOffset(MethodGenerationContext& mgenc) {
+    Emit1(mgenc, BC_JUMP_IF_GREATER, 0);
+    size_t idx = mgenc.AddBytecodeArgumentAndGetIndex(0);
+    mgenc.AddBytecodeArgument(0);
+    return idx;
+}
+
 void EmitJumpBackwardWithOffset(MethodGenerationContext& mgenc,
                                 size_t jumpOffset) {
     uint8_t jumpBytecode =

src/compiler/BytecodeGenerator.h

@@ -58,9 +58,13 @@ void EmitSUPERSEND(MethodGenerationContext& mgenc, VMSymbol* msg);
 void EmitRETURNLOCAL(MethodGenerationContext& mgenc);
 void EmitRETURNNONLOCAL(MethodGenerationContext& mgenc);
 
+void EmitINC(MethodGenerationContext& mgenc);
+void EmitDupSecond(MethodGenerationContext& mgenc);
+
 size_t EmitJumpOnBoolWithDummyOffset(MethodGenerationContext& mgenc,
                                      bool isIfTrue, bool needsPop);
 size_t EmitJumpWithDumyOffset(MethodGenerationContext& mgenc);
+size_t EmitJumpIfGreaterWithDummyOffset(MethodGenerationContext& mgenc);
 void EmitJumpBackwardWithOffset(MethodGenerationContext& mgenc,
                                 size_t jumpOffset);
 size_t Emit3WithDummy(MethodGenerationContext& mgenc, uint8_t bytecode,

src/compiler/LexicalScope.h

@@ -20,6 +20,14 @@ class LexicalScope {
         locals.push_back(var);
     }
 
+    const Variable* GetArgument(size_t index, size_t contextLevel) {
+        if (contextLevel > 0) {
+            return outer->GetArgument(index, contextLevel - 1);
+        }
+
+        return &arguments.at(index);
+    }
+
     const Variable* GetLocal(size_t index, uint8_t ctxLevel) {
         if (ctxLevel > 0) {
             return outer->GetLocal(index, ctxLevel - 1);

src/compiler/MethodGenerationContext.cpp

@@ -439,26 +439,71 @@ bool MethodGenerationContext::InlineAndOr(bool isOr) {
     return true;
 }
 
+bool MethodGenerationContext::InlineToDo() {
+    // HACK: We do assume that the receiver on the stack is a integer,
+    // HACK: similar to the other inlined messages.
+    // HACK: We don't support anything but integer at the moment.
+    assert(Bytecode::GetBytecodeLength(BC_PUSH_BLOCK) == 2);
+    if (!hasOneLiteralBlockArgument()) {
+        return false;
+    }
+
+    VMMethod* toBeInlined =
+        static_cast<VMMethod*>(extractBlockMethodAndRemoveBytecode());
+
+    toBeInlined->MergeScopeInto(*this);
+
+    const Variable* blockArg = toBeInlined->GetArgument(1, 0);
+    uint8_t iVarIdx = GetInlinedLocalIdx(blockArg);
+
+    isCurrentlyInliningABlock = true;
+    EmitDupSecond(*this);
+
+    size_t loopBeginIdx = OffsetOfNextInstruction();
+    size_t jumpOffsetIdxToEnd = EmitJumpIfGreaterWithDummyOffset(*this);
+
+    EmitDUP(*this);
+
+    EmitPOPLOCAL(*this, iVarIdx, 0);
+
+    toBeInlined->InlineInto(*this, false);
+
+    EmitPOP(*this);
+    EmitINC(*this);
+
+    EmitBackwardsJumpOffsetToTarget(loopBeginIdx);
+
+    PatchJumpOffsetToPointToNextInstruction(jumpOffsetIdxToEnd);
+
+    isCurrentlyInliningABlock = false;
+
+    return true;
+}
+
 void MethodGenerationContext::CompleteLexicalScope() {
     lexicalScope = new LexicalScope(
         outerGenc == nullptr ? nullptr : outerGenc->lexicalScope, arguments,
         locals);
 }
 
 void MethodGenerationContext::MergeIntoScope(LexicalScope& scopeToBeInlined) {
-    assert(scopeToBeInlined.GetNumberOfArguments() == 1);
-    size_t numLocals = scopeToBeInlined.GetNumberOfLocals();
-    if (numLocals > 0) {
-        inlineLocals(scopeToBeInlined);
+    if (scopeToBeInlined.GetNumberOfArguments() > 1) {
+        inlineAsLocals(scopeToBeInlined.arguments);
+    }
+
+    if (scopeToBeInlined.GetNumberOfLocals() > 0) {
+        inlineAsLocals(scopeToBeInlined.locals);
     }
 }
 
-void MethodGenerationContext::inlineLocals(LexicalScope& scopeToBeInlined) {
-    for (const Variable& local : scopeToBeInlined.locals) {
-        Variable freshCopy = local.CopyForInlining(this->locals.size());
+void MethodGenerationContext::inlineAsLocals(vector<Variable>& vars) {
+    for (const Variable& var : vars) {
+        Variable freshCopy = var.CopyForInlining(this->locals.size());
         if (freshCopy.IsValid()) {
+            assert(!freshCopy.IsArgument());
+
             // freshCopy can be invalid, because we don't need the $blockSelf
-            std::string qualifiedName = local.MakeQualifiedName();
+            std::string qualifiedName = var.MakeQualifiedName();
             assert(!Contains(this->locals, qualifiedName));
             lexicalScope->AddInlinedLocal(freshCopy);
             this->locals.push_back(freshCopy);

src/compiler/MethodGenerationContext.h

@@ -97,6 +97,7 @@ class MethodGenerationContext {
     bool InlineIfTrueOrIfFalse(bool isIfTrue);
     bool InlineIfTrueFalse(bool isIfTrue);
     bool InlineAndOr(bool isOr);
+    bool InlineToDo();
 
     inline size_t OffsetOfNextInstruction() { return bytecode.size(); }
 
@@ -120,7 +121,7 @@ class MethodGenerationContext {
 
     void completeJumpsAndEmitReturningNil(Parser& parser, size_t loopBeginIdx,
                                           size_t jumpOffsetIdxToSkipLoopBody);
-    void inlineLocals(LexicalScope& scopeToBeInlined);
+    void inlineAsLocals(vector<Variable>& vars);
     void checkJumpOffset(size_t jumpOffset, uint8_t bytecode);
     void resetLastBytecodeBuffer();
 

src/compiler/Parser.cpp

@@ -598,8 +598,8 @@ void Parser::binaryMessage(MethodGenerationContext& mgenc, bool super) {
 
     binaryOperand(mgenc);
 
-    if (!super && (msgSelector == "||" && mgenc.InlineAndOr(true)) ||
-        (msgSelector == "&&" && mgenc.InlineAndOr(false))) {
+    if (!super && ((msgSelector == "||" && mgenc.InlineAndOr(true)) ||
+                   (msgSelector == "&&" && mgenc.InlineAndOr(false)))) {
         return;
     }
 
@@ -645,7 +645,8 @@ void Parser::keywordMessage(MethodGenerationContext& mgenc, bool super) {
 
         if (numParts == 2 &&
             ((kw == "ifTrue:ifFalse:" && mgenc.InlineIfTrueFalse(true)) ||
-             (kw == "ifFalse:ifTrue:" && mgenc.InlineIfTrueFalse(false)))) {
+             (kw == "ifFalse:ifTrue:" && mgenc.InlineIfTrueFalse(false)) ||
+             (kw == "to:do:" && mgenc.InlineToDo()))) {
             return;
         }
     }

src/compiler/Variable.cpp

@@ -21,9 +21,10 @@ std::string Variable::MakeQualifiedName() const {
 Variable Variable::CopyForInlining(size_t newIndex) const {
     if (isArgument) {
         if (name == strBlockSelf) {
+            // that's invalid
             return Variable();
         }
-        return Variable(this, newIndex, true);
     }
+    // arguments that are inlined need to turn into variables, too
     return Variable(this, newIndex, false);
 }

src/compiler/Variable.h

@@ -34,6 +34,8 @@ class Variable {
 
     Variable CopyForInlining(size_t newIndex) const;
 
+    bool IsArgument() const { return isArgument; }
+
 protected:
     std::string name;
     uint8_t index;

src/interpreter/Interpreter.cpp

@@ -34,6 +34,7 @@
 #include "../interpreter/bytecodes.h"  // NOLINT(misc-include-cleaner) it's required for InterpreterLoop.h
 #include "../memory/Heap.h"
 #include "../misc/defs.h"
+#include "../vm/Globals.h"
 #include "../vm/IsValidObject.h"
 #include "../vm/Universe.h"
 #include "../vmobjects/IntegerBox.h"
@@ -42,6 +43,7 @@
 #include "../vmobjects/VMArray.h"
 #include "../vmobjects/VMBlock.h"
 #include "../vmobjects/VMClass.h"
+#include "../vmobjects/VMDouble.h"
 #include "../vmobjects/VMFrame.h"
 #include "../vmobjects/VMInvokable.h"
 #include "../vmobjects/VMMethod.h"
@@ -407,6 +409,38 @@ void Interpreter::doReturnNonLocal() {
     popFrameAndPushResult(result);
 }
 
+void Interpreter::doInc() {
+    vm_oop_t val = GetFrame()->Top();
+
+    if (IS_TAGGED(val) || CLASS_OF(val) == load_ptr(integerClass)) {
+        int64_t result = (int64_t)INT_VAL(val) + 1;
+        val = NEW_INT(result);
+    } else if (CLASS_OF(val) == load_ptr(doubleClass)) {
+        double d = static_cast<VMDouble*>(val)->GetEmbeddedDouble();
+        val = GetUniverse()->NewDouble(d + 1.0);
+    } else {
+        GetUniverse()->ErrorExit("unsupported");
+    }
+
+    GetFrame()->SetTop(store_root(val));
+}
+
+bool Interpreter::checkIsGreater() {
+    vm_oop_t top = GetFrame()->Top();
+    vm_oop_t top2 = GetFrame()->Top2();
+
+    if ((IS_TAGGED(top) || CLASS_OF(top) == load_ptr(integerClass)) &&
+        (IS_TAGGED(top2) || CLASS_OF(top2) == load_ptr(integerClass))) {
+        return INT_VAL(top) > INT_VAL(top2);
+    } else if ((CLASS_OF(top) == load_ptr(doubleClass)) &&
+               (CLASS_OF(top2) == load_ptr(doubleClass))) {
+        return static_cast<VMDouble*>(top)->GetEmbeddedDouble() >
+               static_cast<VMDouble*>(top2)->GetEmbeddedDouble();
+    }
+
+    return false;
+}
+
 void Interpreter::WalkGlobals(walk_heap_fn walk) {
     method = load_ptr(static_cast<GCMethod*>(walk(tmp_ptr(method))));
 

src/interpreter/Interpreter.h

@@ -106,6 +106,8 @@ class Interpreter {
     void doSuperSend(long bytecodeIndex);
     void doReturnLocal();
     void doReturnNonLocal();
+    void doInc();
+    bool checkIsGreater();
 };
 
 inline VMFrame* Interpreter::GetFrame() const {