Fix: Do not generate bci translation logic if tags provided but disabled; transition continuations to cached even if bytecode not updatable

Author: DSouzaM

truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/generator/BytecodeRootNodeElement.java

@@ -768,12 +768,20 @@ private CodeExecutableElement createContinueAt() {
         b.lineComment("Bytecode or tier changed");
         b.tree(GeneratorUtils.createTransferToInterpreterAndInvalidate());
 
-        if (model.isBytecodeUpdatable()) {
+        if (model.isBytecodeUpdatable() || model.hasYieldOperation()) {
             b.declaration(abstractBytecodeNode.asType(), "oldBytecode", "bc");
             b.statement("bc = this.bytecode");
-            b.startAssign("state").startCall("oldBytecode.transitionState");
+
+            if (model.isBytecodeUpdatable()) {
+                b.startAssign("state");
+            } else {
+                b.startStatement();
+            }
+            b.startCall("oldBytecode.transition");
             b.string("bc");
-            b.string("state");
+            if (model.isBytecodeUpdatable()) {
+                b.string("state");
+            }
             if (model.hasYieldOperation()) {
                 b.string("continuationRootNode");
             }
@@ -11772,8 +11780,10 @@ final class AbstractBytecodeNodeElement extends CodeTypeElement {
             }
 
             this.add(createTranslateBytecodeIndex());
+            if (model.isBytecodeUpdatable() || model.hasYieldOperation()) {
+                this.add(createTransition());
+            }
             if (model.isBytecodeUpdatable()) {
-                this.add(createTransitionState());
                 this.add(createToStableBytecodeIndex());
                 this.add(createFromStableBytecodeIndex());
                 this.add(createTransitionInstrumentationIndex());
@@ -12612,7 +12622,7 @@ private CodeExecutableElement createTranslateBytecodeIndex() {
             if (model.isBytecodeUpdatable()) {
 
                 CodeTreeBuilder tb = CodeTreeBuilder.createBuilder();
-                tb.startCall("transitionState");
+                tb.startCall("transition");
                 tb.startGroup();
                 tb.cast(this.asType());
                 tb.string("newNode");
@@ -12632,18 +12642,20 @@ private CodeExecutableElement createTranslateBytecodeIndex() {
             return ex;
         }
 
-        private CodeExecutableElement createTransitionState() {
-            CodeExecutableElement transitionState = new CodeExecutableElement(Set.of(FINAL), type(long.class), "transitionState");
-            transitionState.addParameter(new CodeVariableElement(this.asType(), "newBytecode"));
-            transitionState.addParameter(new CodeVariableElement(type(long.class), "state"));
+        private CodeExecutableElement createTransition() {
+            // Returns updated state long, if updatable.
+            TypeMirror returnType = model.isBytecodeUpdatable() ? type(long.class) : type(void.class);
+
+            CodeExecutableElement ex = new CodeExecutableElement(Set.of(FINAL), returnType, "transition");
+            ex.addParameter(new CodeVariableElement(this.asType(), "newBytecode"));
+            if (model.isBytecodeUpdatable()) {
+                ex.addParameter(new CodeVariableElement(type(long.class), "state"));
+            }
             if (model.hasYieldOperation()) {
-                transitionState.addParameter(new CodeVariableElement(continuationRootNodeImpl.asType(), "continuationRootNode"));
+                ex.addParameter(new CodeVariableElement(continuationRootNodeImpl.asType(), "continuationRootNode"));
             }
 
-            CodeTreeBuilder b = transitionState.createBuilder();
-
-            b.declaration(arrayOf(type(byte.class)), "oldBc", "this.oldBytecodes");
-            b.declaration(arrayOf(type(byte.class)), "newBc", "newBytecode.bytecodes");
+            CodeTreeBuilder b = ex.createBuilder();
 
             if (model.hasYieldOperation()) {
                 /*
@@ -12657,7 +12669,7 @@ private CodeExecutableElement createTransitionState() {
                  * patches all ContinuationRootNodes with the new bytecode, we don't have to update
                  * anything.
                  */
-                b.startIf().string("continuationRootNode != null && oldBc == null").end().startBlock();
+                b.startIf().string("continuationRootNode != null && this.getTier() == ").staticReference(types.BytecodeTier, "UNCACHED").end().startBlock();
                 b.lineComment("Transition continuationRootNode to cached.");
 
                 b.startDeclaration(types.BytecodeLocation, "newContinuationLocation");
@@ -12675,34 +12687,38 @@ private CodeExecutableElement createTransitionState() {
                 b.end();
             }
 
-            b.startIf().string("oldBc == null || this == newBytecode || this.bytecodes == newBc").end().startBlock();
-            b.lineComment("No change in bytecodes.");
-            b.startReturn().string("state").end();
-            b.end();
+            if (model.isBytecodeUpdatable()) {
+                b.declaration(arrayOf(type(byte.class)), "oldBc", "this.oldBytecodes");
+                b.declaration(arrayOf(type(byte.class)), "newBc", "newBytecode.bytecodes");
+                b.startIf().string("oldBc == null || this == newBytecode || this.bytecodes == newBc").end().startBlock();
+                b.lineComment("No change in bytecodes.");
+                b.startReturn().string("state").end();
+                b.end();
 
-            b.declaration(type(int.class), "oldBci", decodeBci("state"));
+                b.declaration(type(int.class), "oldBci", decodeBci("state"));
 
-            b.startDeclaration(type(int.class), "newBci");
-            b.startCall("computeNewBci").string("oldBci").string("oldBc").string("newBc");
-            if (model.enableTagInstrumentation) {
-                b.string("this.getTagNodes()");
-                b.string("newBytecode.getTagNodes()");
-            }
-            b.end(); // call
+                b.startDeclaration(type(int.class), "newBci");
+                b.startCall("computeNewBci").string("oldBci").string("oldBc").string("newBc");
+                if (model.enableTagInstrumentation) {
+                    b.string("this.getTagNodes()");
+                    b.string("newBytecode.getTagNodes()");
+                }
+                b.end(); // call
 
-            b.end();
+                b.end();
 
-            if (model.overridesBytecodeDebugListenerMethod("onBytecodeStackTransition")) {
-                b.startStatement();
-                b.startCall("getRoot().onBytecodeStackTransition");
-                emitParseInstruction(b, "this", "oldBci", readInstruction("oldBc", "oldBci"));
-                emitParseInstruction(b, "newBytecode", "newBci", readInstruction("newBc", "newBci"));
-                b.end().end();
-            }
+                if (model.overridesBytecodeDebugListenerMethod("onBytecodeStackTransition")) {
+                    b.startStatement();
+                    b.startCall("getRoot().onBytecodeStackTransition");
+                    emitParseInstruction(b, "this", "oldBci", readInstruction("oldBc", "oldBci"));
+                    emitParseInstruction(b, "newBytecode", "newBci", readInstruction("newBc", "newBci"));
+                    b.end().end();
+                }
 
-            b.startReturn().string(encodeNewBci("newBci", "state")).end();
+                b.startReturn().string(encodeNewBci("newBci", "state")).end();
+            }
 
-            return transitionState;
+            return ex;
         }
 
         private CodeExecutableElement createTransitionInstrumentationIndex() {
@@ -12740,19 +12756,21 @@ public int compareTo(InstructionGroup o) {
                 }
             }
 
-            CodeExecutableElement invalidate = new CodeExecutableElement(Set.of(PRIVATE, STATIC), type(int.class), "transitionInstrumentationIndex");
-            invalidate.addParameter(new CodeVariableElement(arrayOf(type(byte.class)), "oldBc"));
-            invalidate.addParameter(new CodeVariableElement(type(int.class), "oldBciBase"));
-            invalidate.addParameter(new CodeVariableElement(type(int.class), "oldBciTarget"));
-            invalidate.addParameter(new CodeVariableElement(arrayOf(type(byte.class)), "newBc"));
-            invalidate.addParameter(new CodeVariableElement(type(int.class), "newBciBase"));
+            CodeExecutableElement ex = new CodeExecutableElement(Set.of(PRIVATE, STATIC), type(int.class), "transitionInstrumentationIndex");
+            ex.addParameter(new CodeVariableElement(arrayOf(type(byte.class)), "oldBc"));
+            ex.addParameter(new CodeVariableElement(type(int.class), "oldBciBase"));
+            ex.addParameter(new CodeVariableElement(type(int.class), "oldBciTarget"));
+            ex.addParameter(new CodeVariableElement(arrayOf(type(byte.class)), "newBc"));
+            ex.addParameter(new CodeVariableElement(type(int.class), "newBciBase"));
             if (model.enableTagInstrumentation) {
-                invalidate.addParameter(new CodeVariableElement(arrayOf(tagNode.asType()), "oldTagNodes"));
-                invalidate.addParameter(new CodeVariableElement(arrayOf(tagNode.asType()), "newTagNodes"));
+                ex.addParameter(new CodeVariableElement(arrayOf(tagNode.asType()), "oldTagNodes"));
+                ex.addParameter(new CodeVariableElement(arrayOf(tagNode.asType()), "newTagNodes"));
             }
-            CodeTreeBuilder b = invalidate.createBuilder();
+            CodeTreeBuilder b = ex.createBuilder();
             b.declaration(type(int.class), "oldBci", "oldBciBase");
             b.declaration(type(int.class), "newBci", "newBciBase");
+            b.lineComment("Find the last instrumentation instruction executed before oldBciTarget.");
+            b.lineComment("The new bci should point directly after this reference instruction in the new bytecode.");
             b.declaration(type(short.class), "searchOp", "-1");
             if (model.enableTagInstrumentation) {
                 b.declaration(type(int.class), "searchTags", "-1");
@@ -12765,7 +12783,7 @@ public int compareTo(InstructionGroup o) {
             for (var groupEntry : groupInstructionsSortedBy(InstructionGroup::new)) {
                 InstructionGroup group = groupEntry.getKey();
                 if (!group.instrumentation) {
-                    // seeing an instrumentation here is a failure
+                    // only instrumentation instructions should be reached
                     continue;
                 }
                 List<InstructionModel> instructions = groupEntry.getValue();
@@ -12795,7 +12813,8 @@ public int compareTo(InstructionGroup o) {
             b.end(); // while block
 
             b.startAssert().string("searchOp != -1").end();
-
+            b.lineComment("The instruction may occur multiple times between oldBci and oldTargetBci.");
+            b.lineComment("Count the number of occurrences so that we identify the correct reference instruction.");
             b.startAssign("oldBci").string("oldBciBase").end();
             b.declaration(type(int.class), "opCounter", "0");
 
@@ -12805,7 +12824,7 @@ public int compareTo(InstructionGroup o) {
             for (var groupEntry : groupInstructionsSortedBy(InstructionGroup::new)) {
                 InstructionGroup group = groupEntry.getKey();
                 if (!group.instrumentation) {
-                    // seeing an instrumentation here is a failure
+                    // only instrumentation instructions should be reached
                     continue;
                 }
                 List<InstructionModel> instructions = groupEntry.getValue();
@@ -12839,14 +12858,14 @@ public int compareTo(InstructionGroup o) {
             b.end(); // while block
 
             b.startAssert().string("opCounter > 0").end();
-
+            b.lineComment("Walk the new bytecode to find the location directly after the reference instruction.");
             b.startWhile().string("opCounter > 0").end().startBlock();
             b.declaration(type(short.class), "op", readInstruction("newBc", "newBci"));
             b.startSwitch().string("op").end().startBlock();
             for (var groupEntry : groupInstructionsSortedBy(InstructionGroup::new)) {
                 InstructionGroup group = groupEntry.getKey();
                 if (!group.instrumentation) {
-                    // seeing an instrumentation here is a failure
+                    // only instrumentation instructions should be reached
                     continue;
                 }
                 List<InstructionModel> instructions = groupEntry.getValue();
@@ -12881,7 +12900,7 @@ public int compareTo(InstructionGroup o) {
 
             b.startReturn().string("newBci").end();
 
-            return invalidate;
+            return ex;
         }
 
         private CodeExecutableElement createComputeNewBci() {

truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/model/BytecodeDSLModel.java

@@ -271,7 +271,7 @@ public TypeMirror getProvidedRootBodyTag() {
     }
 
     public boolean isBytecodeUpdatable() {
-        return !getInstrumentations().isEmpty() || !getProvidedTags().isEmpty();
+        return !getInstrumentations().isEmpty() || (enableTagInstrumentation && !getProvidedTags().isEmpty());
     }
 
     public boolean hasYieldOperation() {