Skip to content

Commit

Permalink
[GR-58717] Do not ignore null edges in graphio BinaryReader
Browse files Browse the repository at this point in the history 
PullRequest: graal/18981
  • Loading branch information
@c-refice
c-refice committed Oct 9, 2024
2 parents c69a606 + 92b8272 commit ecbac71
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 98 deletions.
14 changes: 5 additions & 9 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graphio/parsing/BinaryReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1047,26 +1047,22 @@ private void parseBlocks() throws IOException {
}
}

private void createEdges(int id, int preds, List<? extends Port> portList, EdgeBuilder factory) throws IOException {
private void createEdges(int id, int startNum, List<? extends Port> portList, EdgeBuilder factory) throws IOException {
int portNum = 0;
for (Port p : portList) {
if (p.isList) {
int size = dataSource.readShort();
for (int j = 0; j < size; j++) {
int in = dataSource.readInt();
p.ids.add(in);
if (in >= 0) {
factory.edge(p, in, id, (char) (preds + portNum), j);
portNum++;
}
factory.edge(p, in, id, (char) (startNum + portNum), j);
portNum++;
}
} else {
int in = dataSource.readInt();
p.ids.add(in);
if (in >= 0) {
factory.edge(p, in, id, (char) (preds + portNum), -1);
portNum++;
}
factory.edge(p, in, id, (char) (startNum + portNum), -1);
portNum++;
}
p.ids = new ArrayList<>();
}
Expand Down
53 changes: 34 additions & 19 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graphio/parsing/ModelBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,20 +87,22 @@ public static final class EdgeInfo {
final int from;
final int to;
final char num;
final int listIndex;
final String label;
final String type;
final boolean input;

EdgeInfo(int from, int to) {
this(from, to, (char) 0, null, null, false);
this(from, to, (char) 0, -1, null, null, false);
}

EdgeInfo(int from, int to, char num, String label, String type, boolean input) {
EdgeInfo(int from, int to, char num, int listIndex, String label, String type, boolean input) {
this.from = from;
this.to = to;
this.label = label;
this.type = type;
this.num = num;
this.listIndex = listIndex;
this.input = input;
}

Expand All @@ -112,6 +114,10 @@ public int getTo() {
return to;
}

public int getListIndex() {
return listIndex;
}

@Override
public boolean equals(Object other) {
if (other == null) {
Expand All @@ -121,12 +127,13 @@ public boolean equals(Object other) {
return false;
}
EdgeInfo otherEdge = (EdgeInfo) other;
return from == otherEdge.from && to == otherEdge.to && num == otherEdge.num && input == otherEdge.input && Objects.equals(label, otherEdge.label) && Objects.equals(type, otherEdge.type);
return from == otherEdge.from && to == otherEdge.to && num == otherEdge.num && listIndex == otherEdge.listIndex && input == otherEdge.input && Objects.equals(label, otherEdge.label) &&
Objects.equals(type, otherEdge.type);
}

@Override
public int hashCode() {
return Objects.hash(from, to, label);
return Objects.hash(from, to, listIndex, label);
}
}

Expand Down Expand Up @@ -335,10 +342,9 @@ protected final InputGraph pushGraph(InputGraph g) {
}

protected void connectModifiedProperties(FolderElement g) {
if (!(g instanceof Properties.MutableOwner)) {
if (!(g instanceof Properties.MutableOwner<?> mu)) {
return;
}
Properties.MutableOwner<?> mu = (Properties.MutableOwner<?>) g;
GraphDocument doc = g.getOwner();
if (doc != null) {
Properties props = doc.getModifiedProperties(g);
Expand All @@ -357,8 +363,8 @@ public static String makeGraphName(int dumpId, String format, Object[] args) {
for (int i = 0; i < args.length; ++i) {
if (args[i] instanceof BinaryReader.Klass) {
String className = args[i].toString();
String s = className.substring(className.lastIndexOf(".") + 1); // strip the package
// name
// strip the package name
String s = className.substring(className.lastIndexOf(".") + 1);
int innerClassPos = s.indexOf('$');
if (innerClassPos > 0) {
/* Remove inner class name. */
Expand Down Expand Up @@ -721,8 +727,7 @@ private String createName(NodeClass nodeClass, List<EdgeInfo> edges, String temp
String length = m.group(4);
if (prop == null) {
result = "?";
} else if (length != null && prop instanceof LengthToString) {
LengthToString lengthProp = (LengthToString) prop;
} else if (length != null && prop instanceof LengthToString lengthProp) {
switch (length) {
case "s":
result = lengthProp.toString(Length.S);
Expand Down Expand Up @@ -777,27 +782,37 @@ public void setNodeProperty(String key, Object value) {
}
}

protected static String inputEdgeType(Port p) {
EnumValue type = ((TypedPort) p).type;
return type == null ? null : type.toString(Length.S);
}

@Override
public void inputEdge(Port p, int from, int to, char num, int index) {
assert currentNode != null;
String label = (p.isList && index >= 0) ? p.name + "[" + index + "]" : p.name;
EnumValue type = ((TypedPort) p).type;
EdgeInfo ei = new EdgeInfo(from, to, num, label, type == null ? null : type.toString(Length.S), true);
// Ignore null edges
if (from < 0) {
return;
}
EdgeInfo ei = new EdgeInfo(from, to, num, index, p.name, inputEdgeType(p), true);
inputEdges.add(ei);
nodeEdges.add(ei);
}

@Override
public void successorEdge(Port p, int from, int to, char num, int index) {
assert currentNode != null;
String label = (p.isList && index >= 0) ? p.name + "[" + index + "]" : p.name;
EdgeInfo ei = new EdgeInfo(to, from, num, label, InputEdge.SUCCESSOR_EDGE_TYPE, false);
// Ignore null edges
if (from < 0) {
return;
}
EdgeInfo ei = new EdgeInfo(to, from, num, index, p.name, InputEdge.SUCCESSOR_EDGE_TYPE, false);
successorEdges.add(ei);
nodeEdges.add(ei);
}

protected InputEdge immutableEdge(char fromIndex, char toIndex, int from, int to, String label, String type) {
return InputEdge.createImmutable(fromIndex, toIndex, from, to, label, type);
protected InputEdge immutableEdge(char fromIndex, char toIndex, int from, int to, int listIndex, String label, String type) {
return InputEdge.createImmutable(fromIndex, toIndex, from, to, listIndex, label, type);
}

@Override
Expand All @@ -818,13 +833,13 @@ public void makeGraphEdges() {
if (currentGraph.getNode(e.to) == null) {
reportLoadingError(ErrorMessages.edgeEndNodeNotExists(e.label, e.to));
}
graph.addEdge(immutableEdge(fromIndex, toIndex, e.from, e.to, e.label, e.type));
graph.addEdge(immutableEdge(fromIndex, toIndex, e.from, e.to, e.listIndex, e.label, e.type));
}
for (EdgeInfo e : inputEdges) {
assert e.input;
char fromIndex = (char) (nodesWithSuccessor.contains(graph.getNode(e.from)) ? 1 : 0);
char toIndex = e.num;
graph.addEdge(immutableEdge(fromIndex, toIndex, e.from, e.to, e.label, e.type));
graph.addEdge(immutableEdge(fromIndex, toIndex, e.from, e.to, e.listIndex, e.label, e.type));
}
}

Expand Down
44 changes: 30 additions & 14 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graphio/parsing/model/InputEdge.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import java.lang.ref.WeakReference;
import java.util.Comparator;
import java.util.Objects;
import java.util.WeakHashMap;

public class InputEdge {
Expand All @@ -44,10 +45,13 @@ public enum State {

private final char toIndex;
private final char fromIndex;

private final int from;
private final int to;
private final int listIndex;
private final String label;
private final String type;

private State state;
private int hashCode = -1;

Expand All @@ -60,13 +64,17 @@ public InputEdge(char fromIndex, char toIndex, int from, int to) {
}

public InputEdge(char fromIndex, char toIndex, int from, int to, String label, String type) {
this(fromIndex, toIndex, from, to, label, type, State.SAME);
this(fromIndex, toIndex, from, to, -1, label, type, State.SAME);
}

public InputEdge(char fromIndex, char toIndex, int from, int to, int listIndex, String label, String type) {
this(fromIndex, toIndex, from, to, listIndex, label, type, State.SAME);
}

static WeakHashMap<InputEdge, WeakReference<InputEdge>> immutableCache = new WeakHashMap<>();

public static synchronized InputEdge createImmutable(char fromIndex, char toIndex, int from, int to, String label, String type) {
InputEdge edge = new InputEdge(fromIndex, toIndex, from, to, label, type, State.IMMUTABLE);
public static synchronized InputEdge createImmutable(char fromIndex, char toIndex, int from, int to, int listIndex, String label, String type) {
InputEdge edge = new InputEdge(fromIndex, toIndex, from, to, listIndex, label, type, State.IMMUTABLE);
WeakReference<InputEdge> result = immutableCache.get(edge);
if (result != null) {
InputEdge edge2 = result.get();
Expand All @@ -78,18 +86,19 @@ public static synchronized InputEdge createImmutable(char fromIndex, char toInde
return edge;
}

public InputEdge(char fromIndex, char toIndex, int from, int to, String label, String type, State state) {
public InputEdge(char fromIndex, char toIndex, int from, int to, int listIndex, String label, String type, State state) {
this.toIndex = toIndex;
this.fromIndex = fromIndex;
this.from = from;
this.to = to;
this.listIndex = listIndex;
this.state = state;
this.label = label;
this.type = type;

int hash = (from << 20 | to << 8 | toIndex << 4 | fromIndex);
int hash = Objects.hash(from, to, fromIndex, toIndex, listIndex);
if (state == State.IMMUTABLE) {
hash = hash << 5 ^ label.hashCode();
hash = Objects.hash(hash, label);
}
this.hashCode = hash;
}
Expand All @@ -108,7 +117,7 @@ public void setState(State x) {
this.state = x;
// terminal state
if (state == State.IMMUTABLE) {
hashCode = hashCode << 5 ^ label.hashCode();
hashCode = Objects.hash(hashCode, label);
}
}

Expand All @@ -120,10 +129,6 @@ public char getFromIndex() {
return fromIndex;
}

public String getName() {
return "in" + toIndex;
}

public int getFrom() {
return from;
}
Expand All @@ -132,21 +137,32 @@ public int getTo() {
return to;
}

public int getListIndex() {
return listIndex;
}

public String getLabel() {
return label;
}

public String getDisplayLabel() {
String ret = getLabel();
if (listIndex >= 0) {
ret = ret + "[" + listIndex + "]";
}
return ret;
}

public String getType() {
return type;
}

@Override
public boolean equals(Object o) {
if (!(o instanceof InputEdge)) {
if (!(o instanceof InputEdge conn2)) {
return false;
}
InputEdge conn2 = (InputEdge) o;
boolean result = conn2.fromIndex == fromIndex && conn2.toIndex == toIndex && conn2.from == from && conn2.to == to;
boolean result = conn2.fromIndex == fromIndex && conn2.toIndex == toIndex && conn2.from == from && conn2.to == to && conn2.listIndex == listIndex;
if (result && (state == State.IMMUTABLE || conn2.state == State.IMMUTABLE)) {
// Immutable instances must be exactly the same
return conn2.state == state && conn2.label.equals(label);
Expand Down
14 changes: 5 additions & 9 deletions ...alGraphVisualizer/Data/src/main/java/jdk/graal/compiler/graphio/parsing/BinaryReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1047,26 +1047,22 @@ private void parseBlocks() throws IOException {
}
}

private void createEdges(int id, int preds, List<? extends Port> portList, EdgeBuilder factory) throws IOException {
private void createEdges(int id, int startNum, List<? extends Port> portList, EdgeBuilder factory) throws IOException {
int portNum = 0;
for (Port p : portList) {
if (p.isList) {
int size = dataSource.readShort();
for (int j = 0; j < size; j++) {
int in = dataSource.readInt();
p.ids.add(in);
if (in >= 0) {
factory.edge(p, in, id, (char) (preds + portNum), j);
portNum++;
}
factory.edge(p, in, id, (char) (startNum + portNum), j);
portNum++;
}
} else {
int in = dataSource.readInt();
p.ids.add(in);
if (in >= 0) {
factory.edge(p, in, id, (char) (preds + portNum), -1);
portNum++;
}
factory.edge(p, in, id, (char) (startNum + portNum), -1);
portNum++;
}
p.ids = new ArrayList<>();
}
Expand Down
Loading

0 comments on commit ecbac71

Please sign in to comment.