Skip to content

Commit de6b1ec

Browse files
DSouzaMDSouzaM
committed
Fix: Bytecode DSL prepareForCompilation should delegate to super impl when available
1 parent bc273c9 commit de6b1ec

File tree

6 files changed

+252
-25
lines changed

6 files changed

+252
-25
lines changed

‎truffle/src/com.oracle.truffle.api.bytecode.test/src/com/oracle/truffle/api/bytecode/test/RootNodeOverridesTest.java‎

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package com.oracle.truffle.api.bytecode.test;
42+
43+
import static org.junit.Assert.assertEquals;
44+
45+
import org.junit.Test;
46+
47+
import com.oracle.truffle.api.bytecode.BytecodeConfig;
48+
import com.oracle.truffle.api.bytecode.BytecodeParser;
49+
import com.oracle.truffle.api.bytecode.BytecodeRootNode;
50+
import com.oracle.truffle.api.bytecode.BytecodeRootNodes;
51+
import com.oracle.truffle.api.bytecode.BytecodeTier;
52+
import com.oracle.truffle.api.bytecode.ContinuationResult;
53+
import com.oracle.truffle.api.bytecode.ContinuationRootNode;
54+
import com.oracle.truffle.api.bytecode.GenerateBytecode;
55+
import com.oracle.truffle.api.bytecode.Operation;
56+
import com.oracle.truffle.api.dsl.Specialization;
57+
import com.oracle.truffle.api.frame.FrameDescriptor;
58+
import com.oracle.truffle.api.nodes.RootNode;
59+
60+
public class RootNodeOverridesTest {
61+
62+
private static final BytecodeDSLTestLanguage LANGUAGE = null;
63+
64+
private static RootNodeWithOverrides parseRootNodeWithOverrides(BytecodeParser<RootNodeWithOverridesGen.Builder> builder) {
65+
BytecodeRootNodes<RootNodeWithOverrides> nodes = RootNodeWithOverridesGen.create(LANGUAGE, BytecodeConfig.DEFAULT, builder);
66+
return nodes.getNode(0);
67+
}
68+
69+
private static RootNodeWithParentOverrides parseRootNodeWithParentOverrides(BytecodeParser<RootNodeWithParentOverridesGen.Builder> builder) {
70+
BytecodeRootNodes<RootNodeWithParentOverrides> nodes = RootNodeWithParentOverridesGen.create(LANGUAGE, BytecodeConfig.DEFAULT, builder);
71+
return nodes.getNode(0);
72+
}
73+
74+
@Test
75+
public void testPrepareForCompilation() {
76+
RootNodeWithOverrides root = parseRootNodeWithOverrides(b -> {
77+
b.beginRoot();
78+
b.beginReturn();
79+
b.emitLoadConstant(42L);
80+
b.endReturn();
81+
b.endRoot();
82+
});
83+
84+
// As long as the interpreter is uncached, it should not be ready.
85+
assertEquals(BytecodeTier.UNCACHED, root.getBytecodeNode().getTier());
86+
root.readyForCompilation = false;
87+
assertEquals(false, root.prepareForCompilation(true, 2, true));
88+
root.readyForCompilation = true;
89+
assertEquals(false, root.prepareForCompilation(true, 2, true));
90+
91+
// Transition to cached.
92+
root.getBytecodeNode().setUncachedThreshold(0);
93+
root.getCallTarget().call();
94+
assertEquals(BytecodeTier.CACHED, root.getBytecodeNode().getTier());
95+
96+
// When the interpreter is cached, the parent impl should be delegated to.
97+
root.readyForCompilation = false;
98+
assertEquals(false, root.prepareForCompilation(true, 2, true));
99+
root.readyForCompilation = true;
100+
assertEquals(true, root.prepareForCompilation(true, 2, true));
101+
}
102+
103+
@Test
104+
public void testPrepareForCompilationContinuation() {
105+
RootNodeWithOverrides root = parseRootNodeWithOverrides(b -> {
106+
b.beginRoot();
107+
b.beginYield();
108+
b.emitLoadConstant(42L);
109+
b.endYield();
110+
b.beginReturn();
111+
b.emitLoadConstant(42L);
112+
b.endReturn();
113+
b.endRoot();
114+
});
115+
116+
// As long as the interpreter is uncached, it should not be ready.
117+
ContinuationResult cont = (ContinuationResult) root.getCallTarget().call();
118+
assertEquals(BytecodeTier.UNCACHED, root.getBytecodeNode().getTier());
119+
root.readyForCompilation = false;
120+
assertEquals(false, invokePrepareForCompilation(cont.getContinuationRootNode()));
121+
root.readyForCompilation = true;
122+
assertEquals(false, invokePrepareForCompilation(cont.getContinuationRootNode()));
123+
124+
// Transition to cached.
125+
root.getBytecodeNode().setUncachedThreshold(0);
126+
cont = (ContinuationResult) root.getCallTarget().call();
127+
assertEquals(BytecodeTier.CACHED, root.getBytecodeNode().getTier());
128+
129+
// When the interpreter is cached, the parent impl should be delegated to.
130+
root.readyForCompilation = false;
131+
assertEquals(false, invokePrepareForCompilation(cont.getContinuationRootNode()));
132+
root.readyForCompilation = true;
133+
assertEquals(true, invokePrepareForCompilation(cont.getContinuationRootNode()));
134+
}
135+
136+
@Test
137+
public void testPrepareForCompilationContinuationParentOverrides() {
138+
RootNodeWithParentOverrides root = parseRootNodeWithParentOverrides(b -> {
139+
b.beginRoot();
140+
b.beginYield();
141+
b.emitLoadConstant(42L);
142+
b.endYield();
143+
b.beginReturn();
144+
b.emitLoadConstant(42L);
145+
b.endReturn();
146+
b.endRoot();
147+
});
148+
149+
ContinuationResult cont = (ContinuationResult) root.getCallTarget().call();
150+
root.readyForCompilation = false;
151+
assertEquals(false, invokePrepareForCompilation(cont.getContinuationRootNode()));
152+
root.readyForCompilation = true;
153+
assertEquals(true, invokePrepareForCompilation(cont.getContinuationRootNode()));
154+
}
155+
156+
private static boolean invokePrepareForCompilation(ContinuationRootNode cont) {
157+
try {
158+
return (boolean) cont.getClass().getMethod("prepareForCompilation", boolean.class, int.class, boolean.class).invoke(cont, true, 2, true);
159+
} catch (ReflectiveOperationException ex) {
160+
throw new AssertionError("Exception invoking prepareForCompilation", ex);
161+
}
162+
}
163+
164+
@GenerateBytecode(languageClass = BytecodeDSLTestLanguage.class, enableYield = true, enableUncachedInterpreter = true)
165+
abstract static class RootNodeWithOverrides extends RootNode implements BytecodeRootNode {
166+
public boolean readyForCompilation = false;
167+
168+
protected RootNodeWithOverrides(BytecodeDSLTestLanguage language, FrameDescriptor frameDescriptor) {
169+
super(language, frameDescriptor);
170+
}
171+
172+
@Operation
173+
public static final class Add {
174+
@Specialization
175+
static int add(int x, int y) {
176+
return x + y;
177+
}
178+
}
179+
180+
// The generated code should delegate to this method.
181+
@Override
182+
public boolean prepareForCompilation(boolean rootCompilation, int compilationTier, boolean lastTier) {
183+
return readyForCompilation;
184+
}
185+
186+
}
187+
188+
@GenerateBytecode(languageClass = BytecodeDSLTestLanguage.class, enableYield = true)
189+
abstract static class RootNodeWithParentOverrides extends ParentClassWithOverride implements BytecodeRootNode {
190+
191+
protected RootNodeWithParentOverrides(BytecodeDSLTestLanguage language, FrameDescriptor frameDescriptor) {
192+
super(language, frameDescriptor);
193+
}
194+
195+
@Operation
196+
public static final class Add {
197+
@Specialization
198+
static int add(int x, int y) {
199+
return x + y;
200+
}
201+
}
202+
203+
// This root node has no uncached interpreter, so there should be no override. The
204+
// continuation root node should still delegate to the parent implementation.
205+
}
206+
207+
abstract static class ParentClassWithOverride extends RootNode {
208+
public boolean readyForCompilation = false;
209+
210+
protected ParentClassWithOverride(BytecodeDSLTestLanguage language, FrameDescriptor frameDescriptor) {
211+
super(language, frameDescriptor);
212+
}
213+
214+
@Override
215+
public boolean prepareForCompilation(boolean rootCompilation, int compilationTier, boolean lastTier) {
216+
return readyForCompilation;
217+
}
218+
}
219+
220+
}

‎truffle/src/com.oracle.truffle.api.bytecode.test/src/com/oracle/truffle/api/bytecode/test/error_tests/ErrorTests.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,8 +367,9 @@ protected BadOverrides(ErrorLanguage language, FrameDescriptor frameDescriptor)
367367
super(language, frameDescriptor);
368368
}
369369

370-
private static final String ERROR_MESSAGE = "This method is overridden by the generated Bytecode DSL class, so it cannot be declared final. " +
371-
"You can remove the final modifier to resolve this issue, but since the override will make this method unreachable, it is recommended to simply remove it.";
370+
private static final String ERROR_MESSAGE_DELEGATED = "This method is overridden by the generated Bytecode DSL class, so it cannot be declared final.";
371+
private static final String ERROR_MESSAGE = ERROR_MESSAGE_DELEGATED +
372+
" You can remove the final modifier to resolve this issue, but since the override will make this method unreachable, it is recommended to simply remove it.";
372373

373374
@ExpectError(ERROR_MESSAGE)
374375
@Override
@@ -411,7 +412,7 @@ public final boolean isCaptureFramesForTrace(boolean compiledFrame) {
411412
public final void prepareForCall() {
412413
}
413414

414-
@ExpectError(ERROR_MESSAGE)
415+
@ExpectError(ERROR_MESSAGE_DELEGATED)
415416
@Override
416417
public final boolean prepareForCompilation(boolean rootCompilation, int compilationTier, boolean lastTier) {
417418
return false;

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,9 +1665,19 @@ private CodeExecutableElement createPrepareForCompilation() {
16651665
}
16661666

16671667
CodeExecutableElement ex = overrideImplementRootNodeMethod(model, "prepareForCompilation", new String[]{"rootCompilation", "compilationTier", "lastTier"});
1668+
ex.getModifiers().add(Modifier.FINAL);
16681669
CodeTreeBuilder b = ex.createBuilder();
1670+
1671+
b.startReturn();
16691672
// Disable compilation for the uncached interpreter.
1670-
b.startReturn().string("bytecode.getTier() != ").staticReference(types.BytecodeTier, "UNCACHED").end();
1673+
b.string("bytecode.getTier() != ").staticReference(types.BytecodeTier, "UNCACHED");
1674+
1675+
ExecutableElement parentImpl = ElementUtils.findOverride(ElementUtils.findMethod(types.RootNode, "prepareForCompilation", 3), model.templateType);
1676+
if (parentImpl != null) {
1677+
// Delegate to the parent impl.
1678+
b.string(" && ").startCall("super.prepareForCompilation").variables(ex.getParameters()).end();
1679+
}
1680+
b.end();
16711681
return ex;
16721682
}
16731683

@@ -17859,8 +17869,8 @@ private List<CodeExecutableElement> createRootNodeProxyMethods() {
1785917869
continue outer;
1786017870
}
1786117871
}
17862-
// Only proxy methods overridden by the template class.
17863-
ExecutableElement templateMethod = ElementUtils.findOverride(model.templateType, rootNodeMethod);
17872+
// Only proxy methods overridden by the template class or its parents.
17873+
ExecutableElement templateMethod = ElementUtils.findOverride(rootNodeMethod, model.templateType);
1786417874
if (templateMethod == null) {
1786517875
continue outer;
1786617876
}

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

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -385,15 +385,7 @@ public boolean overridesBytecodeDebugListenerMethod(String methodName) {
385385
throw new IllegalArgumentException("Method with name " + methodName + " not found.");
386386
}
387387

388-
TypeElement type = getTemplateType();
389-
while (type != null) {
390-
if (ElementUtils.findOverride(type, e) != null) {
391-
return true;
392-
}
393-
type = ElementUtils.castTypeElement(type.getSuperclass());
394-
}
395-
return false;
396-
388+
return ElementUtils.findOverride(e, getTemplateType()) != null;
397389
}
398390

399391
private InstructionModel instruction(InstructionKind kind, String name, Signature signature, String quickeningName) {

‎truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/parser/BytecodeDSLParser.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,6 @@ private void parseBytecodeDSLModel(TypeElement typeElement, BytecodeDSLModel mod
388388
ElementUtils.findMethod(types.RootNode, "isInstrumentable"),
389389
ElementUtils.findMethod(types.RootNode, "isCaptureFramesForTrace"),
390390
ElementUtils.findMethod(types.RootNode, "prepareForCall"),
391-
ElementUtils.findMethod(types.RootNode, "prepareForCompilation"),
392391
ElementUtils.findMethod(types.RootNode, "prepareForInstrumentation"),
393392
ElementUtils.findMethod(types.BytecodeRootNode, "getBytecodeNode"),
394393
ElementUtils.findMethod(types.BytecodeRootNode, "getRootNodes"),
@@ -413,6 +412,20 @@ private void parseBytecodeDSLModel(TypeElement typeElement, BytecodeDSLModel mod
413412
}
414413
}
415414

415+
List<ExecutableElement> overridesWithDelegation = new ArrayList<>(List.of(
416+
ElementUtils.findMethod(types.RootNode, "prepareForCompilation")));
417+
418+
for (ExecutableElement override : overridesWithDelegation) {
419+
ExecutableElement declared = ElementUtils.findMethod(typeElement, override.getSimpleName().toString());
420+
if (declared == null) {
421+
continue;
422+
}
423+
424+
if (declared.getModifiers().contains(Modifier.FINAL)) {
425+
model.addError(declared, "This method is overridden by the generated Bytecode DSL class, so it cannot be declared final.");
426+
}
427+
}
428+
416429
if (model.hasErrors()) {
417430
return;
418431
}

‎truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/java/ElementUtils.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1834,15 +1834,6 @@ public static boolean isOverride(ExecutableElement e1, ExecutableElement e2) {
18341834
return signatureEquals(e1, e2);
18351835
}
18361836

1837-
public static ExecutableElement findOverride(TypeElement subclass, ExecutableElement method) {
1838-
for (ExecutableElement subclassMethod : ElementFilter.methodsIn(subclass.getEnclosedElements())) {
1839-
if (ElementUtils.isOverride(subclassMethod, method)) {
1840-
return subclassMethod;
1841-
}
1842-
}
1843-
return null;
1844-
}
1845-
18461837
public static boolean elementEquals(Element element1, Element element2) {
18471838
if (element1 == element2) {
18481839
return true;

0 commit comments

Comments
 (0)