diff --git a/src/jdk.incubator.code/share/classes/jdk/incubator/code/bytecode/BytecodeGenerator.java b/src/jdk.incubator.code/share/classes/jdk/incubator/code/bytecode/BytecodeGenerator.java index b449bfd656c..8dde7e35661 100644 --- a/src/jdk.incubator.code/share/classes/jdk/incubator/code/bytecode/BytecodeGenerator.java +++ b/src/jdk.incubator.code/share/classes/jdk/incubator/code/bytecode/BytecodeGenerator.java @@ -63,6 +63,7 @@ import jdk.incubator.code.dialect.core.VarType; import jdk.incubator.code.extern.DialectFactory; import jdk.incubator.code.internal.OpBuilder; +import jdk.incubator.code.internal.RemoveUnusedConstantTransformer; import jdk.incubator.code.runtime.ReflectableLambdaMetafactory; import static java.lang.constant.ConstantDescs.*; @@ -174,8 +175,10 @@ private static byte[] generateClassData(MethodHand BitSet reflectableLambda = new BitSet(); CodeTransformer lowering = LoweringTransform.getInstance(lookup); for (var e : ops.sequencedEntrySet()) { + Op transformed = ConstantExpressionTransformer.transform(lookup, e.getValue()); + transformed = transformed.transform(CodeContext.create(), RemoveUnusedConstantTransformer.INSTANCE); O lowered = NormalizeBlocksTransformer.transform( - (O)e.getValue().transform(CodeContext.create(), lowering)); + (O)transformed.transform(CodeContext.create(), lowering)); generateMethod(lookup, clName, e.getKey(), lowered, clb, ops, lambdaSink, reflectableLambda); } var modelsToBuild = new LinkedHashMap(); diff --git a/src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/java/ConstantExpressionTransformer.java b/src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/java/ConstantExpressionTransformer.java new file mode 100644 index 00000000000..156ca862213 --- /dev/null +++ b/src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/java/ConstantExpressionTransformer.java @@ -0,0 +1,39 @@ +package jdk.incubator.code.dialect.java; + +import jdk.incubator.code.Block; +import jdk.incubator.code.CodeContext; +import jdk.incubator.code.CodeTransformer; +import jdk.incubator.code.Op; + +import java.lang.invoke.MethodHandles; +import java.util.Optional; + +import static jdk.incubator.code.dialect.core.CoreOp.constant; +import static jdk.incubator.code.dialect.java.JavaOp.JavaExpression.ConstantExpressionEvaluator; + +/** + * A transformer that replace every operation that model a constant expression with its value. + */ +public class ConstantExpressionTransformer implements CodeTransformer { + private final ConstantExpressionEvaluator constantExpressionEvaluator; + + private ConstantExpressionTransformer(ConstantExpressionEvaluator constantExpressionEvaluator) { + this.constantExpressionEvaluator = constantExpressionEvaluator; + } + + @Override + public Block.Builder acceptOp(Block.Builder b, Op op) { + Optional v = constantExpressionEvaluator.evaluate(op.result()); + if (v.isPresent()) { + Op.Result c = b.op(constant(op.resultType(), v.get())); + b.context().mapValue(op.result(), c); + } else { + b.op(op); + } + return b; + } + + public static Op transform(MethodHandles.Lookup l, Op op) { + return op.transform(CodeContext.create(), new ConstantExpressionTransformer(new ConstantExpressionEvaluator(l))); + } +} diff --git a/src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/java/JavaOp.java b/src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/java/JavaOp.java index a47c29a00bf..dc916385901 100644 --- a/src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/java/JavaOp.java +++ b/src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/java/JavaOp.java @@ -143,12 +143,7 @@ public sealed interface JavaExpression permits *} */ static Optional evaluate(MethodHandles.Lookup l, Value v) { - try { - Object o = eval(l, v); - return Optional.of(o); - } catch (NonConstantExpression e) { - return Optional.empty(); - } + return new ConstantExpressionEvaluator(l).evaluate(v); } /** @@ -180,151 +175,176 @@ static Optional evaluate(MethodHandles.Lookup l, Value v) { * @jls 15.29 Constant Expressions */ static Optional evaluate(MethodHandles.Lookup l, T op) { - try { - Object v = eval(l, op); - return Optional.of(v); - } catch (NonConstantExpression e) { - return Optional.empty(); - } + return new ConstantExpressionEvaluator(l).evaluate(op); } - private static Object eval(MethodHandles.Lookup l, Op op) { - return switch (op) { - case CoreOp.ConstantOp cop when isConstant(cop) -> { - Object v = cop.value(); - yield v instanceof String s ? s.intern() : v; + class ConstantExpressionEvaluator { + private final MethodHandles.Lookup l; + private final Map m = new HashMap<>(); + + ConstantExpressionEvaluator(MethodHandles.Lookup l) { + this.l = l; + } + + Optional evaluate(T op) { + try { + Object v = this.eval(op); + return Optional.of(v); + } catch (NonConstantExpression e) { + return Optional.empty(); } - case CoreOp.VarAccessOp.VarLoadOp varLoadOp when isConstant(varLoadOp.varOp()) -> - eval(l, varLoadOp.varOp().initOperand()); - case JavaOp.ConvOp _ -> { - // we expect cast to primitive type - var v = eval(l, op.operands().getFirst()); - yield ArithmeticAndConvOpImpls.evaluate(op, List.of(v)); + } + + Optional evaluate(Value v) { + try { + Object o = this.eval(v); + return Optional.of(o); + } catch (NonConstantExpression e) { + return Optional.empty(); } - case CastOp castOp -> { - // we expect cast to String - Value operand = castOp.operands().getFirst(); - if (!castOp.resultType().equals(J_L_STRING) || !operand.type().equals(J_L_STRING)) { - throw new NonConstantExpression(); + } + + private Object eval(Op op) { + if (m.containsKey(op.result())) { + return m.get(op.result()); + } + Object r = switch (op) { + case ConstantOp cop when isConstant(cop) -> { + Object v = cop.value(); + yield v instanceof String s ? s.intern() : v; } - Object v = eval(l, operand); - if (!(v instanceof String s)) { - throw new NonConstantExpression(); + case VarAccessOp.VarLoadOp varLoadOp when varLoadOp.operands().getFirst() instanceof Result && + isConstant(varLoadOp.varOp()) -> eval(varLoadOp.varOp().initOperand()); + case ConvOp _ -> { + // we expect cast to primitive type + var v = eval(op.operands().getFirst()); + yield ArithmeticAndConvOpImpls.evaluate(op, List.of(v)); } - yield s; - } - case ConcatOp concatOp -> { - Object first = eval(l, concatOp.operands().getFirst()); - Object second = eval(l, concatOp.operands().getLast()); - yield (first.toString() + second).intern(); - } - case JavaOp.FieldAccessOp.FieldLoadOp fieldLoadOp -> { - Field field; - VarHandle vh; - try { - field = fieldLoadOp.fieldReference().resolveToField(l); - vh = fieldLoadOp.fieldReference().resolveToHandle(l); - } catch (ReflectiveOperationException e) { - throw new IllegalArgumentException(e); + case CastOp castOp -> { + // we expect cast to String + Value operand = castOp.operands().getFirst(); + if (!castOp.resultType().equals(J_L_STRING) || !operand.type().equals(J_L_STRING)) { + throw new NonConstantExpression(); + } + Object v = eval(operand); + if (!(v instanceof String s)) { + throw new NonConstantExpression(); + } + yield s; } - // Requirement: the field must be a constant variable. - // Current checks: - // 1) The field is declared final. - // 2) The field type is a primitive or String. - // Missing check: - // 3) Verify the field is initialized and the initializer is a constant expression. - if ((field.getModifiers() & Modifier.FINAL) == 0 || - !isConstantType(fieldLoadOp.fieldReference().type())) { - throw new NonConstantExpression(); + case ConcatOp concatOp -> { + Object first = eval(concatOp.operands().getFirst()); + Object second = eval(concatOp.operands().getLast()); + yield (first.toString() + second).intern(); } - Object v; - if ((field.getModifiers() & Modifier.STATIC) != 0) { - // @@@ why using field.get fails ? - v = vh.get(); - } else { - // we can't get the value of an instance field from the model - // we need the value of the receiver - throw new NonConstantExpression(); + case FieldAccessOp.FieldLoadOp fieldLoadOp -> { + Field field; + VarHandle vh; + try { + field = fieldLoadOp.fieldReference().resolveToField(l); + vh = fieldLoadOp.fieldReference().resolveToHandle(l); + } catch (ReflectiveOperationException e) { + throw new IllegalArgumentException(e); + } + // Requirement: the field must be a constant variable. + // Current checks: + // 1) The field is declared final. + // 2) The field type is a primitive or String. + // Missing check: + // 3) Verify the field is initialized and the initializer is a constant expression. + if ((field.getModifiers() & Modifier.FINAL) == 0 || + !isConstantType(fieldLoadOp.fieldReference().type())) { + throw new NonConstantExpression(); + } + Object v; + if ((field.getModifiers() & Modifier.STATIC) != 0) { + v = vh.get(); + } else { + // we can't get the value of an instance field from the model + // we need the value of the receiver + throw new NonConstantExpression(); + } + yield v instanceof String s ? s.intern() : v; } - yield v instanceof String s ? s.intern() : v; - } - case ArithmeticOperation _ -> { - List values = op.operands().stream().map(v -> eval(l, v)).toList(); - yield ArithmeticAndConvOpImpls.evaluate(op, values); - } - case JavaOp.ConditionalExpressionOp _ -> { - boolean p = evalBoolean(l, op.bodies().get(0)); - Object t = eval(l, op.bodies().get(1)); - Object f = eval(l, op.bodies().get(2)); - yield p ? t : f; - } - case JavaOp.ConditionalAndOp _ -> { - boolean left = evalBoolean(l, op.bodies().get(0)); - boolean right = evalBoolean(l, op.bodies().get(1)); - yield left && right; - } - case JavaOp.ConditionalOrOp _ -> { - boolean left = evalBoolean(l, op.bodies().get(0)); - boolean right = evalBoolean(l, op.bodies().get(1)); - yield left || right; - } - default -> throw new NonConstantExpression(); - }; - } - - private static Object eval(MethodHandles.Lookup l, Value v) { - if (v.declaringElement() instanceof JavaExpression e) { - return eval(l, (Op & JavaExpression) e); + case ArithmeticOperation _ -> { + List values = op.operands().stream().map(this::eval).toList(); + yield ArithmeticAndConvOpImpls.evaluate(op, values); + } + case ConditionalExpressionOp _ -> { + boolean p = evalBoolean(op.bodies().get(0)); + Object t = eval(op.bodies().get(1)); + Object f = eval(op.bodies().get(2)); + yield p ? t : f; + } + case ConditionalAndOp _ -> { + boolean left = evalBoolean(op.bodies().get(0)); + boolean right = evalBoolean(op.bodies().get(1)); + yield left && right; + } + case ConditionalOrOp _ -> { + boolean left = evalBoolean(op.bodies().get(0)); + boolean right = evalBoolean(op.bodies().get(1)); + yield left || right; + } + default -> throw new NonConstantExpression(); + }; + m.put(op.result(), r); + return r; } - throw new NonConstantExpression(); - } - private static Object eval(MethodHandles.Lookup l, Body body) throws NonConstantExpression { - if (body.blocks().size() != 1 || - !(body.entryBlock().terminatingOp() instanceof CoreOp.YieldOp yop) || - yop.yieldValue() == null || - !isConstantType(yop.yieldValue().type())) { + private Object eval(Value v) { + if (v.declaringElement() instanceof JavaExpression e) { + return eval((Op & JavaExpression) e); + } throw new NonConstantExpression(); } - return eval(l, yop.yieldValue()); - } - private static boolean evalBoolean(MethodHandles.Lookup l, Body body) throws NonConstantExpression { - Object eval = eval(l, body); - if (!(eval instanceof Boolean b)) { - throw new NonConstantExpression(); + private Object eval(Body body) throws NonConstantExpression { + if (body.blocks().size() != 1 || + !(body.entryBlock().terminatingOp() instanceof CoreOp.YieldOp yop) || + yop.yieldValue() == null || + !isConstantType(yop.yieldValue().type())) { + throw new NonConstantExpression(); + } + return eval(yop.yieldValue()); } - return b; - } + private boolean evalBoolean(Body body) throws NonConstantExpression { + Object eval = eval(body); + if (!(eval instanceof Boolean b)) { + throw new NonConstantExpression(); + } + return b; + } - private static boolean isConstant(CoreOp.ConstantOp op) { - return isConstantType(op.resultType()) && isConstantValue(op.value()); - } + private static boolean isConstant(CoreOp.ConstantOp op) { + return isConstantType(op.resultType()) && isConstantValue(op.value()); + } - private static boolean isConstant(VarOp op) { - // Requirement: the local variable must be a constant variable. - // Current checks: - // 1) The variable is initialized, and the initializer is a constant expression. - // 2) The variable type is a primitive or String. - // Missing check: - // 3) Ensure the variable is declared final - return isConstantType(op.varValueType()) && - !op.isUninitialized() && - // @@@ Add to VarOp - op.result().uses().stream().noneMatch(u -> u.op() instanceof CoreOp.VarAccessOp.VarStoreOp); - } + private static boolean isConstant(VarOp op) { + // Requirement: the local variable must be a constant variable. + // Current checks: + // 1) The variable is initialized, and the initializer is a constant expression. + // 2) The variable type is a primitive or String. + // Missing check: + // 3) Ensure the variable is declared final + return isConstantType(op.varValueType()) && + !op.isUninitialized() && + // @@@ Add to VarOp + op.result().uses().stream().noneMatch(u -> u.op() instanceof CoreOp.VarAccessOp.VarStoreOp); + } - private static boolean isConstantValue(Object o) { - return switch (o) { - case String _ -> true; - case Boolean _, Byte _, Short _, Character _, Integer _, Long _, Float _, Double _ -> true; - case null, default -> false; - }; - } + private static boolean isConstantValue(Object o) { + return switch (o) { + case String _ -> true; + case Boolean _, Byte _, Short _, Character _, Integer _, Long _, Float _, Double _ -> true; + case null, default -> false; + }; + } - private static boolean isConstantType(CodeType e) { - return (e instanceof PrimitiveType && !VOID.equals(e)) || J_L_STRING.equals(e); + private static boolean isConstantType(CodeType e) { + return (e instanceof PrimitiveType && !VOID.equals(e)) || J_L_STRING.equals(e); + } } } diff --git a/src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/RemoveUnusedConstantTransformer.java b/src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/RemoveUnusedConstantTransformer.java new file mode 100644 index 00000000000..22913784a4a --- /dev/null +++ b/src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/RemoveUnusedConstantTransformer.java @@ -0,0 +1,24 @@ +package jdk.incubator.code.internal; + +import jdk.incubator.code.Block; +import jdk.incubator.code.CodeTransformer; +import jdk.incubator.code.Op; +import jdk.incubator.code.dialect.core.CoreOp; + +/** + * A transformer that removes unused {@link jdk.incubator.code.dialect.core.CoreOp.ConstantOp}. + */ +public class RemoveUnusedConstantTransformer implements CodeTransformer { + private RemoveUnusedConstantTransformer() {} + + public static final RemoveUnusedConstantTransformer INSTANCE = new RemoveUnusedConstantTransformer(); + + @Override + public Block.Builder acceptOp(Block.Builder builder, Op op) { + if (op instanceof CoreOp.ConstantOp && op.result() != null && op.result().uses().isEmpty()) { + return builder; + } + builder.op(op); + return builder; + } +} diff --git a/test/jdk/jdk/incubator/code/TestEvaluation.java b/test/jdk/jdk/incubator/code/TestConstantExpressionEvaluation.java similarity index 92% rename from test/jdk/jdk/incubator/code/TestEvaluation.java rename to test/jdk/jdk/incubator/code/TestConstantExpressionEvaluation.java index 953384cbbb3..734f82558c8 100644 --- a/test/jdk/jdk/incubator/code/TestEvaluation.java +++ b/test/jdk/jdk/incubator/code/TestConstantExpressionEvaluation.java @@ -9,8 +9,8 @@ import org.junit.jupiter.params.provider.MethodSource; import java.lang.invoke.MethodHandles; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.lang.reflect.Modifier; import java.time.LocalDate; import java.util.Arrays; import java.util.List; @@ -23,11 +23,11 @@ * @test * @modules jdk.incubator.code * @library lib - * @run junit TestEvaluation - * @run main Unreflect TestEvaluation - * @run junit TestEvaluation + * @run junit TestConstantExpressionEvaluation + * @run main Unreflect TestConstantExpressionEvaluation + * @run junit TestConstantExpressionEvaluation */ -public class TestEvaluation { +public class TestConstantExpressionEvaluation { @Reflect static int primitiveLiteral() { return 1; @@ -122,8 +122,7 @@ static boolean equalityOperator2() { static boolean equalityOperator3() { return "A" != "B"; } - // @Reflect - // @@@ Interpreter doesn't intern constant expression of type String, JDK-8379503 + @Reflect static boolean stringReferenceEquality() { return "A" + "A" == "AA"; } @@ -209,10 +208,9 @@ static int fcFinalVariableNotInitialized() { return i; } - //@Reflect - static int fcEffectivelyFinalVar() { - // @@@ should fail - // currently we lack sufficent info to determine if a variable was declared final in source code + @Reflect + static int effectivelyFinalVar() { + // the op evaluation API broaden the JLS notion of constant variable to include effectively final variable int x = 1; return x; } @@ -267,7 +265,7 @@ static String fcFieldBlankFinal() { @ParameterizedTest @MethodSource("cases") - void test(Method m) throws NoSuchMethodException { + void test(Method m) throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { CoreOp.FuncOp f = Op.ofMethod(m).get(); Op op = ((Op.Result) f.body().entryBlock().terminatingOp().operands().getFirst()).op(); MethodHandles.Lookup l = MethodHandles.lookup(); @@ -276,18 +274,13 @@ void test(Method m) throws NoSuchMethodException { Assertions.assertTrue(v.isEmpty(), m.getName()); } else { Assertions.assertTrue(v.isPresent(), m.getName()); - Object[] args = new Object[0]; - if ((m.getModifiers() & Modifier.STATIC) == 0) { // instance method - args = new Object[] {this}; - } - // TODO use BytecodeGenerator instead of Interpreter - Object expected = Interpreter.invoke(l, f.transform(CodeTransformer.LOWERING_TRANSFORMER), args); + Object expected = m.invoke(null); Assertions.assertEquals(expected, v.get()); } } static Stream cases() { - return Arrays.stream(TestEvaluation.class.getDeclaredMethods()) + return Arrays.stream(TestConstantExpressionEvaluation.class.getDeclaredMethods()) .filter(m -> m.isAnnotationPresent(Reflect.class)); } diff --git a/test/jdk/jdk/incubator/code/TestStringConstantExpressionInterning.java b/test/jdk/jdk/incubator/code/TestStringConstantExpressionInterning.java new file mode 100644 index 00000000000..1152a16ef4e --- /dev/null +++ b/test/jdk/jdk/incubator/code/TestStringConstantExpressionInterning.java @@ -0,0 +1,112 @@ +import jdk.incubator.code.CodeTransformer; +import jdk.incubator.code.Op; +import jdk.incubator.code.Reflect; +import jdk.incubator.code.bytecode.BytecodeGenerator; +import jdk.incubator.code.dialect.java.ConstantExpressionTransformer; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.lang.invoke.MethodHandles; +import java.lang.reflect.Method; +import java.time.LocalDate; +import java.util.Arrays; +import java.util.stream.Stream; + +import static jdk.incubator.code.dialect.core.CoreOp.*; + +/* + * @test + * @modules jdk.incubator.code + * @library lib + * @run junit TestStringConstantExpressionInterning + */ +public class TestStringConstantExpressionInterning { + + @Reflect + static boolean t_strLiteral() { + return "A" + 1 == "A1"; + } + + static final String B = "B"; + @Reflect + static boolean t_classVariable() { + return B + 1 == "B1"; + } + + static final String C; + static { + C = "C"; + } + //@Reflect + static boolean f_classVariable2() { + // C + 1 shouldn't be interned, but currently it's + // that's a limitation in the evaluate API, as the API consider C a constant variable + return C + 1 == "C1"; + } + + static final String D = "D".toLowerCase(); + //@Reflect + static boolean f_classVariable3() { + // D shouldn't be interned, but currently it's + // that's a limitation in the evaluate API, as the API consider D a constant variable + return D == "d"; + } + + final String E = "E"; + //@Reflect + boolean t_instanceVariable() { + // A + 1 should be interned, but currently it's not + // because the op evaluate API considers E (and any instance field) a non constant variable + // it's a limitation in the API + // if core reflection can tag fields that are constant variables the limitation will be solved + // or, we can change the model by replacing read of constant variable by constant + // but the latter only work for compiler generated model + return E + 1 == "E1"; + } + + @Reflect + static boolean t_localVariable() { + // in the model, we broaden the notion of JLS constant variable to include effectively final variable + String s = "A"; + return s + 1 == "A1"; + } + + @Reflect + static boolean t_case7() { + // @@@ we keep the conditional expr in the model + String s = 1 < 2 ? "A" + 1 : LocalDate.now().toString(); + return s == "A1"; + } + + @Reflect + static boolean t_case8() { + int i = 1; + String s = switch(i) { + case 1 -> "A" + 2 + 4; + default -> "B"; + }; + return s == "A24"; + } + + static Stream cases() { + return Arrays.stream(TestStringConstantExpressionInterning.class.getDeclaredMethods()) + .filter(m -> m.isAnnotationPresent(Reflect.class)); + } + + @ParameterizedTest + @MethodSource("cases") + void test(Method m) throws Throwable { + FuncOp op = Op.ofMethod(m).get(); + // expected value is encoded in the method name because evaluation API not fully aligned with JLS + Object expected = m.getName().startsWith("t"); + MethodHandles.Lookup l = MethodHandles.lookup(); + + FuncOp lop = op.transform(CodeTransformer.LOWERING_TRANSFORMER); + FuncOp cfop = (FuncOp) ConstantExpressionTransformer.transform(l, lop); + Assertions.assertEquals(expected, Interpreter.invoke(l, cfop)); + + Assertions.assertEquals(expected, BytecodeGenerator.generate(l, op).invoke()); + } + +}