Skip to content

Commit 3cc9ddb

Browse files
christianhaeublchristianhaeubl
committed
Merge with master.
2 parents a143cf6 + 909560e commit 3cc9ddb

File tree

23 files changed

+1141
-251
lines changed

23 files changed

+1141
-251
lines changed

‎compiler/src/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/MathCopySignStampTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,22 @@ public void testFloatCopySign() throws InvalidInstalledCodeException {
9090
}
9191
}
9292

93+
public static int badCopyStampGR43958(short magnitude) {
94+
/*
95+
* Previously the stamp of the copySign would be incorrect.
96+
*/
97+
return (short) Math.copySign(magnitude, -942.5804f);
98+
}
99+
100+
@Test
101+
public void testGR43958() throws InvalidInstalledCodeException {
102+
short[] shortValues = {42, -42, 95};
103+
for (short magnitude : shortValues) {
104+
InstalledCode code = getCode(getResolvedJavaMethod("badCopyStampGR43958"), null, true);
105+
Assert.assertEquals(badCopyStampGR43958(magnitude), (int) code.executeVarargs(magnitude), 0);
106+
}
107+
}
108+
93109
private static final double[] doubleValues = {
94110
0.0d,
95111
-0.0d,

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/CopySignNode.java‎

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -51,36 +51,52 @@ public CopySignNode(ValueNode magnitude, ValueNode sign) {
5151
super(TYPE, computeStamp(magnitude.stamp(NodeView.DEFAULT), sign.stamp(NodeView.DEFAULT)), magnitude, sign);
5252
}
5353

54-
public static Stamp computeStamp(Stamp stampX, Stamp stampY) {
55-
FloatStamp floatStampX = (FloatStamp) stampX;
56-
FloatStamp floatStampY = (FloatStamp) stampY;
57-
if (floatStampX.isNaN()) {
58-
return stampX;
54+
public static Stamp computeStamp(Stamp magnitude, Stamp sign) {
55+
FloatStamp magnitudeStamp = (FloatStamp) magnitude;
56+
FloatStamp signStamp = (FloatStamp) sign;
57+
if (magnitudeStamp.isNaN()) {
58+
return magnitude;
5959
}
60-
if (floatStampY.isNonNaN()) {
61-
if (floatStampY.lowerBound() > 0) {
62-
if (floatStampX.lowerBound() > 0) {
63-
return floatStampX;
60+
if (signStamp.isNonNaN()) {
61+
if (signStamp.lowerBound() > 0) {
62+
// the end result will be non-negative
63+
if (magnitudeStamp.lowerBound() > 0) {
64+
// We know the entire range is above 0: leave it unchanged.
65+
return magnitudeStamp;
6466
}
65-
if (floatStampX.upperBound() < 0) {
66-
return new FloatStamp(floatStampX.getBits(), -floatStampX.upperBound(), -floatStampX.lowerBound(), floatStampX.isNonNaN());
67+
if (magnitudeStamp.upperBound() < 0) {
68+
// We know that the entire range is below 0
69+
// flip [lower, upper] to [-upper, -lower]
70+
return new FloatStamp(magnitudeStamp.getBits(), -magnitudeStamp.upperBound(), -magnitudeStamp.lowerBound(), magnitudeStamp.isNonNaN());
6771
}
68-
return new FloatStamp(floatStampX.getBits(), Math.min(-floatStampX.lowerBound(), floatStampX.upperBound()),
69-
Math.max(-floatStampX.lowerBound(), floatStampX.upperBound()), floatStampX.isNonNaN());
72+
// We know lowerBound <= 0 and upperBound >= 0:
73+
// the new range is [0, Math.max(-lower, upper)]
74+
return new FloatStamp(magnitudeStamp.getBits(), 0,
75+
Math.max(-magnitudeStamp.lowerBound(), magnitudeStamp.upperBound()), magnitudeStamp.isNonNaN());
7076
}
71-
if (floatStampY.upperBound() < 0) {
72-
if (floatStampX.upperBound() < 0) {
73-
return floatStampX;
77+
if (signStamp.upperBound() < 0) {
78+
// the result will be non-positive
79+
if (magnitudeStamp.upperBound() < 0) {
80+
// We know the entire range is below 0: leave it unchanged.
81+
return magnitudeStamp;
7482
}
75-
if (floatStampX.lowerBound() > 0) {
76-
return new FloatStamp(floatStampX.getBits(), -floatStampX.upperBound(), -floatStampX.lowerBound(), floatStampX.isNonNaN());
83+
if (magnitudeStamp.lowerBound() > 0) {
84+
// We know that the entire range is above 0
85+
// flip [lower, upper] to [-upper,-lower]
86+
return new FloatStamp(magnitudeStamp.getBits(), -magnitudeStamp.upperBound(), -magnitudeStamp.lowerBound(), magnitudeStamp.isNonNaN());
7787
}
78-
return new FloatStamp(floatStampX.getBits(), Math.min(floatStampX.lowerBound(), -floatStampX.upperBound()),
79-
Math.max(floatStampX.lowerBound(), -floatStampX.upperBound()), floatStampX.isNonNaN());
88+
// We know lowerBound <= 0 and upperBound >= 0
89+
// the new range is [Math.min(lower, -upper), 0]
90+
return new FloatStamp(magnitudeStamp.getBits(), Math.min(magnitudeStamp.lowerBound(), -magnitudeStamp.upperBound()),
91+
0, magnitudeStamp.isNonNaN());
8092
}
8193
}
82-
return new FloatStamp(floatStampX.getBits(), Math.min(floatStampX.lowerBound(), -floatStampX.upperBound()), Math.max(-floatStampX.lowerBound(), floatStampX.upperBound()),
83-
floatStampX.isNonNaN());
94+
/*
95+
* We have no information on whether the range will be flipped or not. Hence, we have to
96+
* expand the result to be the union of [lower, upper] and [-upper, -lower].
97+
*/
98+
return new FloatStamp(magnitudeStamp.getBits(), Math.min(magnitudeStamp.lowerBound(), -magnitudeStamp.upperBound()), Math.max(-magnitudeStamp.lowerBound(), magnitudeStamp.upperBound()),
99+
magnitudeStamp.isNonNaN());
84100
}
85101

86102
@Override

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/SignumNode.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ private static Stamp computeStamp(Stamp stamp) {
6767
return new FloatStamp(floatStamp.getBits(), -1.0D, -1.0D, true);
6868
}
6969
}
70-
FloatStamp result = new FloatStamp(floatStamp.getBits(), Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY, !floatStamp.canBeNaN());
70+
// Initially make an empty stamp.
71+
FloatStamp result = new FloatStamp(floatStamp.getBits(), Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY, floatStamp.isNonNaN());
7172
if (floatStamp.contains(0.0d)) {
7273
// this also covers stamp.contains(-0.0d)
7374
result = (FloatStamp) result.meet(new FloatStamp(floatStamp.getBits(), 0.0d, 0.0d, true));

‎compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/OptimizeExtendsPhase.java‎

Lines changed: 8 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,12 @@
2525

2626
package org.graalvm.compiler.phases.common;
2727

28-
import java.util.ArrayList;
2928
import java.util.List;
3029
import java.util.Optional;
3130

3231
import org.graalvm.collections.EconomicMap;
3332
import org.graalvm.collections.EconomicSet;
3433
import org.graalvm.collections.Equivalence;
35-
import org.graalvm.collections.MapCursor;
3634
import org.graalvm.compiler.core.common.memory.MemoryExtendKind;
3735
import org.graalvm.compiler.core.common.type.IntegerStamp;
3836
import org.graalvm.compiler.debug.Assertions;
@@ -50,7 +48,6 @@
5048
import org.graalvm.compiler.nodes.calc.SignExtendNode;
5149
import org.graalvm.compiler.nodes.calc.ZeroExtendNode;
5250
import org.graalvm.compiler.nodes.memory.ExtendableMemoryAccess;
53-
import org.graalvm.compiler.nodes.spi.LoweringProvider;
5451
import org.graalvm.compiler.phases.BasePhase;
5552
import org.graalvm.compiler.phases.tiers.LowTierContext;
5653

@@ -59,7 +56,7 @@
5956
* done by folding extends into memory operations and also by "overextending" extends to cover all
6057
* given use scenarios. In the case of overextension, an original extend may be replaced with an
6158
* "extend+narrow" combo. Note that within the backend code generation narrows are free (via
62-
* {@link org.graalvm.compiler.lir.CastValue}, so it is inconsequential to add more of them. Note
59+
* {@link org.graalvm.compiler.lir.CastValue}), so it is inconsequential to add more of them. Note
6360
* that this optimization will always result in the same or fewer number of extends along all paths.
6461
*
6562
* The idea case is a situation such as
@@ -169,7 +166,6 @@ protected void run(StructuredGraph graph, LowTierContext context) {
169166
* extend as a use. We resolve this via tracking when they change.
170167
*/
171168
EconomicMap<ValueNode, ValueNode> extendReplacements = EconomicMap.create(Equivalence.DEFAULT);
172-
EconomicSet<ValueNode> addedNarrows = Assertions.assertionsEnabled() ? EconomicSet.create(Equivalence.DEFAULT) : null;
173169
/* Step 2: try to optimize extends */
174170
for (ValueNode origDef : defsWithExtends) {
175171

@@ -295,9 +291,6 @@ protected void run(StructuredGraph graph, LowTierContext context) {
295291
if (resultBits != replacementBits) {
296292
assert replacementBits > resultBits;
297293
replacement = graph.addOrUnique(new NarrowNode(replacement, replacementBits, resultBits));
298-
if (Assertions.assertionsEnabled()) {
299-
addedNarrows.add(replacement);
300-
}
301294
}
302295

303296
// replace original extend node
@@ -316,76 +309,14 @@ protected void run(StructuredGraph graph, LowTierContext context) {
316309
}
317310
}
318311

319-
assert validateOptimization(graph, context.getLowerer(), origNumExtends, addedNarrows);
320-
}
321-
322-
/**
323-
* After this optimization is performed:
324-
* <ul>
325-
* <li>The number of extends in the graph should be less than or equal to the number of extends
326-
* in the original graph</li>
327-
* <li>Any given non-extended def should have at most 2 extends attached to it.</li>
328-
* <li>If possible, an extend has been folded into the def</li>
329-
* </ul>
330-
*/
331-
private static boolean validateOptimization(StructuredGraph graph, LoweringProvider lowerer, int origNumExtends, EconomicSet<ValueNode> addedNarrows) {
332-
int numExtends = graph.getNodes().filter(OptimizeExtendsPhase::isExtendNode).count();
333-
assert numExtends <= origNumExtends;
334-
335-
/* Step 1: link extends to their defs */
336-
EconomicMap<ValueNode, List<IntegerConvertNode<?>>> extendMap = EconomicMap.create(Equivalence.DEFAULT);
337-
for (Node node : graph.getNodes().filter(OptimizeExtendsPhase::isExtendNode)) {
338-
IntegerConvertNode<?> extend = (IntegerConvertNode<?>) node;
339-
ValueNode def = extend.getValue();
340-
// Ignore narrows we've added
341-
while (def instanceof NarrowNode && addedNarrows.contains(def)) {
342-
def = ((NarrowNode) def).getValue();
343-
}
344-
if ((def instanceof ExtendableMemoryAccess) && ((ExtendableMemoryAccess) def).extendsAccess()) {
345-
// via the def being extended the correct optimization has already occurred.
346-
continue;
347-
}
348-
List<IntegerConvertNode<?>> value = extendMap.get(def);
349-
if (value == null) {
350-
value = new ArrayList<>();
351-
extendMap.put(def, value);
352-
}
353-
value.add(extend);
354-
}
355-
356-
/*
357-
* Step 2: ensure any non-extended def has at most two extends and nothing can be folded
358-
* into the def.
359-
*/
360-
MapCursor<ValueNode, List<IntegerConvertNode<?>>> entries = extendMap.getEntries();
361-
while (entries.advance()) {
362-
ValueNode def = entries.getKey();
363-
List<IntegerConvertNode<?>> extendNodes = entries.getValue();
364-
assert extendNodes.size() <= 2;
365-
if (extendNodes.size() == 2) {
366-
boolean firstIsZeroExtend = extendNodes.get(0) instanceof ZeroExtendNode;
367-
boolean secondIsZeroExtend = extendNodes.get(1) instanceof ZeroExtendNode;
368-
// the extends should be of the opposite type
369-
assert firstIsZeroExtend ^ secondIsZeroExtend;
370-
}
371-
372-
// Ensure nothing more could be folded into the def.
373-
if (def instanceof ExtendableMemoryAccess) {
374-
ExtendableMemoryAccess access = (ExtendableMemoryAccess) def;
375-
for (IntegerConvertNode<?> extend : extendNodes) {
376-
MemoryExtendKind extendKind;
377-
int extendSize = extend.getResultBits();
378-
if (extend instanceof ZeroExtendNode) {
379-
extendKind = MemoryExtendKind.getZeroExtendKind(extendSize);
380-
} else {
381-
assert extend instanceof SignExtendNode;
382-
extendKind = MemoryExtendKind.getSignExtendKind(extendSize);
383-
}
384-
assert !lowerer.supportsFoldingExtendIntoAccess(access, extendKind);
385-
}
386-
}
312+
if (Assertions.assertionsEnabled()) {
313+
/*
314+
* After this optimization is performed the number of extends in the graph should be
315+
* less than or equal to the number of extends in the original graph.
316+
*/
317+
int numExtends = graph.getNodes().filter(OptimizeExtendsPhase::isExtendNode).count();
318+
assert numExtends <= origNumExtends;
387319
}
388-
return true;
389320
}
390321

391322
private static boolean isExtendNode(Node node) {

‎compiler/src/org.graalvm.util/src/org/graalvm/util/json/JSONFormatter.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -92,6 +92,16 @@ public static String formatJSON(EconomicMap<String, Object> map, boolean indent)
9292
return sb.toString();
9393
}
9494

95+
public static String formatJSON(List<?> elements) {
96+
return formatJSON(elements, false);
97+
}
98+
99+
public static String formatJSON(List<?> elements, boolean indent) {
100+
StringBuilder sb = new StringBuilder();
101+
appendTo(sb, elements, indent ? DEFAULT_INDENT : null, EMPTY_STRING);
102+
return sb.toString();
103+
}
104+
95105
private static String quote(CharSequence value) {
96106
StringBuilder builder = new StringBuilder(value.length() + 2);
97107
builder.append(DOUBLE_QUOTE);
@@ -164,6 +174,9 @@ static void appendTo(StringBuilder sb, List<?> contents, String indent, String c
164174
sb.append(COMMA_SPACE);
165175
}
166176
}
177+
if (indent != null) {
178+
sb.append(newIndent);
179+
}
167180
appendValue(sb, value, indent, newIndent);
168181
comma = true;
169182
}

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/InvokeTypeFlow.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,9 @@ protected void updateReceiver(PointsToAnalysis bb, MethodFlowsGraphInfo calleeFl
177177
if (formalReceiverFlow != null) {
178178
formalReceiverFlow.addReceiverState(bb, receiverTypeState);
179179
}
180+
}
180181

182+
if (bb.getHostVM().getMultiMethodAnalysisPolicy().performReturnLinking(callerMultiMethodKey, calleeFlows.getMethod().getMultiMethodKey())) {
181183
if (bb.optimizeReturnedParameter()) {
182184
int paramIndex = calleeFlows.getMethod().getTypeFlow().getReturnedParameterIndex();
183185
if (actualReturn != null && paramIndex == 0) {

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,6 @@ public Set<Object> getWrittenBy() {
393393
return writtenBy.keySet();
394394
}
395395

396-
private boolean isAccessedSet() {
397-
return AtomicUtils.isSet(this, isAccessedUpdater);
398-
}
399-
400396
/**
401397
* Returns true if the field is reachable. Fields that are read or manually registered as
402398
* reachable are always reachable. For fields that are write-only, more cases need to be
@@ -417,26 +413,14 @@ public boolean isAccessed() {
417413
(AtomicUtils.isSet(this, isWrittenUpdater) && (Modifier.isVolatile(getModifiers()) || getStorageKind() == JavaKind.Object));
418414
}
419415

420-
private boolean isReadSet() {
421-
return AtomicUtils.isSet(this, isReadUpdater);
422-
}
423-
424416
public boolean isRead() {
425417
return AtomicUtils.isSet(this, isAccessedUpdater) || AtomicUtils.isSet(this, isReadUpdater);
426418
}
427419

428-
private boolean isWrittenSet() {
429-
return AtomicUtils.isSet(this, isWrittenUpdater);
430-
}
431-
432420
public boolean isWritten() {
433421
return AtomicUtils.isSet(this, isAccessedUpdater) || AtomicUtils.isSet(this, isWrittenUpdater);
434422
}
435423

436-
private boolean isFoldedSet() {
437-
return AtomicUtils.isSet(this, isFoldedUpdater);
438-
}
439-
440424
public boolean isFolded() {
441425
return AtomicUtils.isSet(this, isFoldedUpdater);
442426
}
@@ -517,7 +501,8 @@ public boolean isStatic() {
517501

518502
@Override
519503
public String toString() {
520-
return "AnalysisField<" + format("%h.%n") + " accessed: " + isAccessedSet() + " reads: " + isReadSet() + " written: " + isWrittenSet() + " folded: " + isFoldedSet() + ">";
504+
return "AnalysisField<" + format("%h.%n") + " -> " + wrapped.toString() + ", accessed: " + (isAccessed != null) +
505+
", read: " + (isRead != null) + ", written: " + (isWritten != null) + ", folded: " + (isFolded != null) + ">";
521506
}
522507

523508
public void markAsUsedInComparison() {

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,9 @@ public LineNumberTable getLineNumberTable() {
685685

686686
@Override
687687
public String toString() {
688-
return "AnalysisMethod<" + format("%H.%n") + " -> " + wrapped.toString() + ">";
688+
return "AnalysisMethod<" + format("%h.%n") + " -> " + wrapped.toString() + ", invoked: " + (isInvoked != null) +
689+
", implInvoked: " + (isImplementationInvoked != null) + ", intrinsic: " + (isIntrinsicMethod != null) + ", inlined: " + (isInlined != null) +
690+
(isVirtualRootMethod != 0 ? ", virtual root" : "") + (isDirectRootMethod != 0 ? ", direct root" : "") + (isEntryPoint() ? ", entry point" : "") + ">";
689691
}
690692

691693
@Override

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav
108108

109109
protected final AnalysisUniverse universe;
110110
private final ResolvedJavaType wrapped;
111+
private final String qualifiedName;
112+
private final String unqualifiedName;
111113

112114
@SuppressWarnings("unused") private volatile Object isInHeap;
113115
@SuppressWarnings("unused") private volatile Object isAllocated;
@@ -210,6 +212,9 @@ public enum UsageKind {
210212
public AnalysisType(AnalysisUniverse universe, ResolvedJavaType javaType, JavaKind storageKind, AnalysisType objectType, AnalysisType cloneableType) {
211213
this.universe = universe;
212214
this.wrapped = javaType;
215+
qualifiedName = wrapped.toJavaName(true);
216+
unqualifiedName = wrapped.toJavaName(false);
217+
213218
isArray = wrapped.isArray();
214219
isJavaLangObject = wrapped.isJavaLangObject();
215220
this.storageKind = storageKind;
@@ -845,6 +850,16 @@ public final String getName() {
845850
return wrapped.getName();
846851
}
847852

853+
@Override
854+
public String toJavaName() {
855+
return qualifiedName;
856+
}
857+
858+
@Override
859+
public String toJavaName(boolean qualified) {
860+
return qualified ? qualifiedName : unqualifiedName;
861+
}
862+
848863
@Override
849864
public final JavaKind getJavaKind() {
850865
return wrapped.getJavaKind();
@@ -1145,7 +1160,8 @@ public String getSourceFileName() {
11451160

11461161
@Override
11471162
public String toString() {
1148-
return "AnalysisType<" + toJavaName(true) + ", allocated: " + isAllocated + ", inHeap: " + isInHeap + ", reachable: " + isReachable + ">";
1163+
return "AnalysisType<" + unqualifiedName + " -> " + wrapped.toString() + ", allocated: " + (isAllocated != null) +
1164+
", inHeap: " + (isInHeap != null) + ", reachable: " + (isReachable != null) + ">";
11491165
}
11501166

11511167
@Override

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/image/ImageHeap.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
public interface ImageHeap {
3030
Collection<? extends ImageHeapObject> getObjects();
3131

32-
ImageHeapObject addLateToImageHeap(Object object, String reason);
32+
ImageHeapObject addLateToImageHeap(Object object, Object reason);
3333

3434
ImageHeapObject addFillerObject(int size);
3535

0 commit comments

Comments
 (0)