Skip to content

Commit f6d89ae

Browse files
authored
Remove constant folding in Painless user tree (#52783)
This change removes constant folding from the Painless user tree. This accomplishes two goals. The first is to remove all optimizations from the user tree which will at a later time become optimization phases for the ir tree. The second is to make the user tree immutable, and this is a step toward that since we will no longer remove/modify/replace nodes in the user tree during the process of constant folding. One important note is that the conditional promotion code has changed, since the promoteConditional method considered constants (similarly to the JVM spec) that are now removed, but this code path was unreachable to begin with so the constants were never actually used to determine the appropriate promotion.
1 parent fb64c18 commit f6d89ae

20 files changed

+85
-555
lines changed

modules/lang-painless/src/main/java/org/elasticsearch/painless/AnalyzerCaster.java

Lines changed: 16 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ public static Class<?> promoteEquality(Class<?> from0, Class<?> from1) {
520520
return Object.class;
521521
}
522522

523-
public static Class<?> promoteConditional(Class<?> from0, Class<?> from1, Object const0, Object const1) {
523+
public static Class<?> promoteConditional(Class<?> from0, Class<?> from1) {
524524
if (from0 == from1) {
525525
return from0;
526526
}
@@ -529,123 +529,29 @@ public static Class<?> promoteConditional(Class<?> from0, Class<?> from1, Object
529529
return def.class;
530530
}
531531

532-
if (from0.isPrimitive() && from1.isPrimitive()) {
533-
if (from0 == boolean.class && from1 == boolean.class) {
534-
return boolean.class;
535-
}
536-
532+
if (from0.isPrimitive() && from0 != boolean.class && from1.isPrimitive() && from1 != boolean.class) {
537533
if (from0 == double.class || from1 == double.class) {
538534
return double.class;
539535
} else if (from0 == float.class || from1 == float.class) {
540536
return float.class;
541537
} else if (from0 == long.class || from1 == long.class) {
542538
return long.class;
539+
} else if (from0 == int.class || from1 == int.class) {
540+
return int.class;
541+
} else if (from0 == char.class) {
542+
if (from1 == short.class || from1 == byte.class) {
543+
return int.class;
544+
} else {
545+
return null;
546+
}
547+
} else if (from1 == char.class) {
548+
if (from0 == short.class || from0 == byte.class) {
549+
return int.class;
543550
} else {
544-
if (from0 == byte.class) {
545-
if (from1 == byte.class) {
546-
return byte.class;
547-
} else if (from1 == short.class) {
548-
if (const1 != null) {
549-
final short constant = (short)const1;
550-
551-
if (constant <= Byte.MAX_VALUE && constant >= Byte.MIN_VALUE) {
552-
return byte.class;
553-
}
554-
}
555-
556-
return short.class;
557-
} else if (from1 == char.class) {
558-
return int.class;
559-
} else if (from1 == int.class) {
560-
if (const1 != null) {
561-
final int constant = (int)const1;
562-
563-
if (constant <= Byte.MAX_VALUE && constant >= Byte.MIN_VALUE) {
564-
return byte.class;
565-
}
566-
}
567-
568-
return int.class;
569-
}
570-
} else if (from0 == short.class) {
571-
if (from1 == byte.class) {
572-
if (const0 != null) {
573-
final short constant = (short)const0;
574-
575-
if (constant <= Byte.MAX_VALUE && constant >= Byte.MIN_VALUE) {
576-
return byte.class;
577-
}
578-
}
579-
580-
return short.class;
581-
} else if (from1 == short.class) {
582-
return short.class;
583-
} else if (from1 == char.class) {
584-
return int.class;
585-
} else if (from1 == int.class) {
586-
if (const1 != null) {
587-
final int constant = (int)const1;
588-
589-
if (constant <= Short.MAX_VALUE && constant >= Short.MIN_VALUE) {
590-
return short.class;
591-
}
592-
}
593-
594-
return int.class;
595-
}
596-
} else if (from0 == char.class) {
597-
if (from1 == byte.class) {
598-
return int.class;
599-
} else if (from1 == short.class) {
600-
return int.class;
601-
} else if (from1 == char.class) {
602-
return char.class;
603-
} else if (from1 == int.class) {
604-
if (const1 != null) {
605-
final int constant = (int)const1;
606-
607-
if (constant <= Character.MAX_VALUE && constant >= Character.MIN_VALUE) {
608-
return byte.class;
609-
}
610-
}
611-
612-
return int.class;
613-
}
614-
} else if (from0 == int.class) {
615-
if (from1 == byte.class) {
616-
if (const0 != null) {
617-
final int constant = (int)const0;
618-
619-
if (constant <= Byte.MAX_VALUE && constant >= Byte.MIN_VALUE) {
620-
return byte.class;
621-
}
622-
}
623-
624-
return int.class;
625-
} else if (from1 == short.class) {
626-
if (const0 != null) {
627-
final int constant = (int)const0;
628-
629-
if (constant <= Short.MAX_VALUE && constant >= Short.MIN_VALUE) {
630-
return byte.class;
631-
}
632-
}
633-
634-
return int.class;
635-
} else if (from1 == char.class) {
636-
if (const0 != null) {
637-
final int constant = (int)const0;
638-
639-
if (constant <= Character.MAX_VALUE && constant >= Character.MIN_VALUE) {
640-
return byte.class;
641-
}
642-
}
643-
644-
return int.class;
645-
} else if (from1 == int.class) {
646-
return int.class;
647-
}
551+
return null;
648552
}
553+
} else {
554+
return null;
649555
}
650556
}
651557

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/AExpression.java

Lines changed: 7 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.painless.ir.ClassNode;
2626
import org.elasticsearch.painless.ir.ExpressionNode;
2727
import org.elasticsearch.painless.lookup.PainlessCast;
28-
import org.elasticsearch.painless.lookup.PainlessLookupUtility;
2928
import org.elasticsearch.painless.symbol.ScriptRoot;
3029

3130
import java.util.Objects;
@@ -84,14 +83,6 @@ public abstract class AExpression extends ANode {
8483
*/
8584
boolean internal = false;
8685

87-
/**
88-
* Set to the value of the constant this expression node represents if
89-
* and only if the node represents a constant. If this is not null
90-
* this node will be replaced by an {@link EConstant} during casting
91-
* if it's not already one.
92-
*/
93-
Object constant = null;
94-
9586
/**
9687
* Set to true by {@link ENull} to represent a null value.
9788
*/
@@ -134,97 +125,14 @@ AExpression cast(ScriptRoot scriptRoot, Scope scope) {
134125
PainlessCast cast = AnalyzerCaster.getLegalCast(location, actual, expected, explicit, internal);
135126

136127
if (cast == null) {
137-
if (constant == null || this instanceof EConstant) {
138-
// For the case where a cast is not required and a constant is not set
139-
// or the node is already an EConstant no changes are required to the tree.
140-
141-
return this;
142-
} else {
143-
// For the case where a cast is not required but a
144-
// constant is set, an EConstant replaces this node
145-
// with the constant copied from this node. Note that
146-
// for constants output data does not need to be copied
147-
// from this node because the output data for the EConstant
148-
// will already be the same.
149-
150-
EConstant econstant = new EConstant(location, constant);
151-
econstant.analyze(scriptRoot, scope);
152-
153-
if (!expected.equals(econstant.actual)) {
154-
throw createError(new IllegalStateException("Illegal tree structure."));
155-
}
156-
157-
return econstant;
158-
}
128+
return this;
159129
} else {
160-
if (constant == null) {
161-
// For the case where a cast is required and a constant is not set.
162-
// Modify the tree to add an ECast between this node and its parent.
163-
// The output data from this node is copied to the ECast for
164-
// further reads done by the parent.
165-
166-
ECast ecast = new ECast(location, this, cast);
167-
ecast.statement = statement;
168-
ecast.actual = expected;
169-
ecast.isNull = isNull;
170-
171-
return ecast;
172-
} else {
173-
if (PainlessLookupUtility.isConstantType(expected)) {
174-
// For the case where a cast is required, a constant is set,
175-
// and the constant can be immediately cast to the expected type.
176-
// An EConstant replaces this node with the constant cast appropriately
177-
// from the constant value defined by this node. Note that
178-
// for constants output data does not need to be copied
179-
// from this node because the output data for the EConstant
180-
// will already be the same.
181-
182-
constant = AnalyzerCaster.constCast(location, constant, cast);
183-
184-
EConstant econstant = new EConstant(location, constant);
185-
econstant.analyze(scriptRoot, scope);
186-
187-
if (!expected.equals(econstant.actual)) {
188-
throw createError(new IllegalStateException("Illegal tree structure."));
189-
}
190-
191-
return econstant;
192-
} else if (this instanceof EConstant) {
193-
// For the case where a cast is required, a constant is set,
194-
// the constant cannot be immediately cast to the expected type,
195-
// and this node is already an EConstant. Modify the tree to add
196-
// an ECast between this node and its parent. Note that
197-
// for constants output data does not need to be copied
198-
// from this node because the output data for the EConstant
199-
// will already be the same.
200-
201-
ECast ecast = new ECast(location, this, cast);
202-
ecast.actual = expected;
203-
204-
return ecast;
205-
} else {
206-
// For the case where a cast is required, a constant is set,
207-
// the constant cannot be immediately cast to the expected type,
208-
// and this node is not an EConstant. Replace this node with
209-
// an Econstant node copying the constant from this node.
210-
// Modify the tree to add an ECast between the EConstant node
211-
// and its parent. Note that for constants output data does not
212-
// need to be copied from this node because the output data for
213-
// the EConstant will already be the same.
214-
215-
EConstant econstant = new EConstant(location, constant);
216-
econstant.analyze(scriptRoot, scope);
217-
218-
if (!actual.equals(econstant.actual)) {
219-
throw createError(new IllegalStateException("Illegal tree structure."));
220-
}
221-
222-
ECast ecast = new ECast(location, econstant, cast);
223-
ecast.actual = expected;
224-
225-
return ecast;
226-
}
227-
}
130+
ECast ecast = new ECast(location, this, cast);
131+
ecast.statement = statement;
132+
ecast.actual = expected;
133+
ecast.isNull = isNull;
134+
135+
return ecast;
228136
}
229137
}
230138
}

0 commit comments

Comments
 (0)