diff --git a/src/main/java/org/prlprg/ir/cfg/Assumption.java b/src/main/java/org/prlprg/ir/cfg/Assumption.java index 53efe6917..fadc85b7d 100644 --- a/src/main/java/org/prlprg/ir/cfg/Assumption.java +++ b/src/main/java/org/prlprg/ir/cfg/Assumption.java @@ -24,6 +24,6 @@ public InstrOrPhi origin() { @Override public NodeId id() { - return new LocalNodeIdImpl<>(this, checkpoint.id(), index); + return new AuxiliaryNodeIdImpl<>(InstrOrPhiIdImpl.cast(checkpoint.id()), index); } } diff --git a/src/main/java/org/prlprg/ir/cfg/BB.java b/src/main/java/org/prlprg/ir/cfg/BB.java index f4dd4c819..ee9d6d14c 100644 --- a/src/main/java/org/prlprg/ir/cfg/BB.java +++ b/src/main/java/org/prlprg/ir/cfg/BB.java @@ -1,5 +1,7 @@ package org.prlprg.ir.cfg; +import static org.prlprg.AppConfig.EAGERLY_VERIFY_PHI_INPUTS; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; @@ -333,7 +335,7 @@ public BB splitNewPredecessor(int index) { if (index < 0 || index > stmts.size()) { throw new IndexOutOfBoundsException("Index out of range: " + index); } - var newBB = cfg().doAddBB(id.name()); + var newBB = cfg().doAddBB(BBIdImpl.cast(id).name()); for (var pred : predecessors) { assert pred.jump != null && pred.jump.targets().contains(this) @@ -356,7 +358,7 @@ public BB splitNewSuccessor(int index) { if (index < 0 || index > stmts.size()) { throw new IndexOutOfBoundsException("Index out of range: " + index); } - var newBB = cfg().doAddBB(id.name()); + var newBB = cfg().doAddBB(BBIdImpl.cast(id).name()); for (var succ : successors()) { assert succ.predecessors.contains(this) @@ -364,7 +366,7 @@ public BB splitNewSuccessor(int index) { succ.predecessors.remove(this); succ.predecessors.add(newBB); for (var phi : succ.phis) { - ((PhiImpl) phi).unsafeReplaceIncomingBB(this, newBB); + PhiImpl.cast(phi).unsafeReplaceIncomingBB(this, newBB); } } @@ -408,7 +410,7 @@ public void mergeWithSuccessor(BB succ) { succ2.predecessors.remove(succ); succ2.predecessors.add(this); for (var phi : succ2.phis) { - ((PhiImpl) phi).unsafeReplaceIncomingBB(succ, this); + PhiImpl.cast(phi).unsafeReplaceIncomingBB(succ, this); } } @@ -432,27 +434,20 @@ public void mergeWithSuccessor(BB succ) { // region add nodes @Override public Phi addPhi(Class nodeClass) { - var phi = - PhiImpl.forClass( - null, nodeClass, cfg(), predecessors.stream().map(Phi.Input::unset).toList()); - phis.add(phi); - cfg().track(phi); - - cfg().record(new CFGEdit.InsertPhi<>(this, phi), new CFGEdit.RemovePhi(this, phi)); - return phi; + return addPhiWithId(cfg().>uniqueInstrOrPhiId(), nodeClass); } @Override public Phi addPhi( Class nodeClass, Collection> inputs) { - return addPhiWithId(null, nodeClass, inputs); + return addPhiWithId(cfg().>uniqueInstrOrPhiId(), nodeClass, inputs); } Phi addPhiWithId( - @Nullable NodeId> id, Class nodeClass) { + NodeId> id, Class nodeClass) { var phi = PhiImpl.forClass( - id, nodeClass, cfg(), predecessors.stream().map(Phi.Input::unset).toList()); + nodeClass, cfg(), id, predecessors.stream().map(Phi.Input::unset).toList()); phis.add(phi); cfg().track(phi); @@ -461,15 +456,22 @@ Phi addPhiWithId( } Phi addPhiWithId( - @Nullable NodeId> id, + NodeId> id, Class nodeClass, Collection> inputs) { - var phi = PhiImpl.forClass(id, nodeClass, cfg(), inputs); + var phi = PhiImpl.forClass(nodeClass, cfg(), id, inputs); verifyPhiInputBBs(phi); phis.add(phi); cfg().track(phi); cfg().record(new CFGEdit.InsertPhi<>(this, phi), new CFGEdit.RemovePhi(this, phi)); + + // Has to be after insertion, and we want to try and not leave the state inconsistent on + // exception (this specific check is allowed to fail in a consistent state). + if (EAGERLY_VERIFY_PHI_INPUTS) { + PhiImpl.cast(phi).eagerlyVerifyInputs(); + } + return phi; } @@ -479,7 +481,9 @@ public ImmutableList> addPhis( var inputs = predecessors.stream().map(Phi.Input::unset).toList(); var phis = nodeClasses.stream() - .map(nodeClass -> PhiImpl.forClass(null, nodeClass, cfg(), inputs)) + .map( + nodeClass -> + PhiImpl.forClass(nodeClass, cfg(), cfg().>uniqueInstrOrPhiId(), inputs)) .collect(ImmutableList.toImmutableList()); this.phis.addAll(phis); cfg().trackAll(phis); @@ -490,13 +494,18 @@ public ImmutableList> addPhis( @Override public ImmutableList> addPhis1(Collection phiArgs) { - return addPhisWithIds(phiArgs.stream().map(PhiImpl.Args::new).toList()); + return addPhisWithIds( + phiArgs.stream() + .map(a -> new PhiImpl.Args(cfg().>uniqueInstrOrPhiId(), a)) + .toList()); } ImmutableList> addPhisWithIds(Collection phiArgs) { var phis = phiArgs.stream() - .map(a -> PhiImpl.forClass(a.id(), a.nodeClass(), cfg(), a.inputs())) + // If the compiler gives you an error here, and IntelliJ says it's fine, you must + // either rebuild, or `mvn clean` and rebuild, and it will magically go away. + .map(a -> PhiImpl.forClass(a.nodeClass(), cfg(), a.id(), a.inputs())) .collect(ImmutableList.toImmutableList()); for (var phi : phis) { verifyPhiInputBBs(phi); @@ -505,6 +514,15 @@ ImmutableList> addPhisWithIds(Collection phi) { @Override public I insertAt(int index, String name, StmtData args) { - return insertAtWithToken(index, new CreateInstrWithNewId(name), args); + return insertAtWithId(index, cfg().uniqueInstrOrPhiId(name), args); } - I insertAtWithToken( - int index, TokenToCreateNewInstr token, StmtData args) { + I insertAtWithId(int index, NodeId id, StmtData args) { if (index < 0 || index > stmts.size()) { throw new IndexOutOfBoundsException("Index out of range: " + index); } - var stmt = args.make(cfg(), token); + var stmt = args.make(cfg(), id); stmts.add(index, stmt); cfg().track(stmt); @@ -542,17 +559,20 @@ I insertAtWithToken( @Override public ImmutableList insertAllAt(int index, List namesAndArgs) { - return insertAllAtWithTokens(index, namesAndArgs.stream().map(StmtImpl.Args::new).toList()); + return insertAllAtWithIds( + index, + namesAndArgs.stream() + .map(a -> new StmtImpl.Args(cfg().uniqueInstrOrPhiId(a.name()), a.data())) + .toList()); } - ImmutableList insertAllAtWithTokens( - int index, List tokensAndArgs) { + ImmutableList insertAllAtWithIds(int index, List idsAndArgs) { if (index < 0 || index > stmts.size()) { throw new IndexOutOfBoundsException("Index out of range: " + index); } var stmts = - tokensAndArgs.stream() - .map(idAndArgs -> idAndArgs.data().make(cfg(), idAndArgs.token())) + idsAndArgs.stream() + .map(idAndArgs -> idAndArgs.data().make(cfg(), idAndArgs.id())) .collect(ImmutableList.toImmutableList()); this.stmts.addAll(index, stmts); cfg().trackAll(stmts); @@ -566,14 +586,19 @@ ImmutableList insertAllAtWithTokens( @Override public ImmutableList insertAllAt(Map indicesNamesAndArgs) { - return insertAllAtWithTokens( + return insertAllAtWithIds( indicesNamesAndArgs.entrySet().stream() - .collect(Collectors.toMap(Map.Entry::getKey, e -> new StmtImpl.Args(e.getValue())))); + .collect( + Collectors.toMap( + Map.Entry::getKey, + e -> + new StmtImpl.Args( + cfg().uniqueInstrOrPhiId(e.getValue().name()), + e.getValue().data())))); } - ImmutableList insertAllAtWithTokens( - Map indicesTokensAndArgs) { - for (var index : indicesTokensAndArgs.keySet()) { + ImmutableList insertAllAtWithIds(Map indicesIdsAndArgs) { + for (var index : indicesIdsAndArgs.keySet()) { if (index < 0 || index > stmts.size()) { throw new IndexOutOfBoundsException("Index out of range: " + index); } @@ -583,14 +608,14 @@ ImmutableList insertAllAtWithTokens( // O(origStmts.size * indicesNamesAndArgs.size) time if we inserted one by one. // Also remember that later indices must not be offset by earlier ones. var origStmts = new ArrayList<>(stmts); - var newStmtsBuilder = ImmutableList.builderWithExpectedSize(indicesTokensAndArgs.size()); + var newStmtsBuilder = ImmutableList.builderWithExpectedSize(indicesIdsAndArgs.size()); var indicesStmts = - ImmutableMap.builderWithExpectedSize(indicesTokensAndArgs.size()); + ImmutableMap.builderWithExpectedSize(indicesIdsAndArgs.size()); stmts.clear(); for (var i = 0; i < origStmts.size(); i++) { - if (indicesTokensAndArgs.containsKey(i)) { - var tokenAndArgs = indicesTokensAndArgs.get(i); - var stmt = tokenAndArgs.data().make(cfg(), tokenAndArgs.token()); + if (indicesIdsAndArgs.containsKey(i)) { + var idAndArgs = indicesIdsAndArgs.get(i); + var stmt = idAndArgs.data().make(cfg(), idAndArgs.id()); stmts.add(stmt); newStmtsBuilder.add(stmt); indicesStmts.put(i, stmt); @@ -611,14 +636,14 @@ ImmutableList insertAllAtWithTokens( @Override public I addJump(String name, JumpData args) { - return addJumpWithToken(new CreateInstrWithNewId(name), args); + return addJumpWithId(cfg().uniqueInstrOrPhiId(name), args); } - I addJumpWithToken(TokenToCreateNewInstr token, JumpData args) { + I addJumpWithId(NodeId id, JumpData args) { if (this.jump != null) { - throw new IllegalStateException(id() + " already has a jump, call replaceJump instead:\n"); + throw new IllegalStateException(id() + " already has a jump, call replaceJump instead"); } - var jump = args.make(cfg(), token); + var jump = args.make(cfg(), id); setJump(jump); cfg().track(jump); @@ -636,19 +661,19 @@ public I replace(int index, @Nullable String newName, StmtData< } var oldStmt = stmts.get(index); if (newName == null) { - newName = oldStmt.id().name(); + newName = InstrOrPhiIdImpl.cast(oldStmt.id()).name(); } - return replaceWithToken(index, new CreateInstrWithNewId(newName), newArgs); + return replaceWithId(index, cfg().uniqueInstrOrPhiId(newName), newArgs); } - I replaceWithToken( - int index, TokenToCreateNewInstr newIdToken, StmtData newArgs) { + I replaceWithId( + int index, NodeId newId, StmtData newArgs) { if (index < 0 || index >= stmts.size()) { throw new IndexOutOfBoundsException("Index out of range: " + index); } var oldStmt = stmts.get(index); - var newStmt = newArgs.make(cfg(), newIdToken); + var newStmt = newArgs.make(cfg(), newId); stmts.set(index, newStmt); cfg().untrack(oldStmt); @@ -668,19 +693,19 @@ public I replaceJump(@Nullable String newName, JumpData newA } var oldJump = jump; if (newName == null) { - newName = oldJump.id().name(); + newName = InstrOrPhiIdImpl.cast(oldJump.id()).name(); } - return replaceJumpWithToken(new CreateInstrWithNewId(newName), newArgs); + return replaceJumpWithId(cfg().uniqueInstrOrPhiId(newName), newArgs); } - public I replaceJumpWithToken( - TokenToCreateNewInstr newIdToken, JumpData newArgs) { + public I replaceJumpWithId( + NodeId newId, JumpData newArgs) { if (jump == null) { throw new IllegalStateException(id() + " doesn't have a jump"); } var oldJump = jump; - var newJump = newArgs.make(cfg(), newIdToken); + var newJump = newArgs.make(cfg(), newId); setJump(newJump); cfg().untrack(oldJump); @@ -791,7 +816,7 @@ public Jump removeJump() { void unsafeAddPredecessor(BB predecessor) { predecessors.add(predecessor); for (var phi : phis) { - ((PhiImpl) phi).unsafeAddUnsetInput(predecessor); + PhiImpl.cast(phi).unsafeAddUnsetInput(predecessor); } } @@ -806,7 +831,7 @@ void unsafeAddPredecessor(BB predecessor) { void unsafeRemovePredecessor(BB predecessor) { predecessors.remove(predecessor); for (var phi : phis) { - ((PhiImpl) phi).unsafeRemoveInput(predecessor); + PhiImpl.cast(phi).unsafeRemoveInput(predecessor); } } diff --git a/src/main/java/org/prlprg/ir/cfg/BBId.java b/src/main/java/org/prlprg/ir/cfg/BBId.java index 4a662e2dd..99273c3c0 100644 --- a/src/main/java/org/prlprg/ir/cfg/BBId.java +++ b/src/main/java/org/prlprg/ir/cfg/BBId.java @@ -11,22 +11,13 @@ * *

Every basic block has an ID unique within its CFG, but not other CFGs. */ -public interface BBId { - /** - * Descriptive name to make the logic in the CFG clearer, or empty string if there is none. - * - *

e.g. "entry", "deopt", "for_loop_start". - * - *

This is not guaranteed to be unique within a CFG. - */ - String name(); - +public sealed interface BBId { @ParseMethod(SkipWhitespace.NONE) private static BBId parse(Parser p) { var s = p.scanner(); s.assertAndSkip('^'); - var disambiguator = s.readUInt(); + var disambiguator = s.nextCharSatisfies(Character::isDigit) ? s.readUInt() : 0; var name = NodeAndBBIds.readName(s); return new BBIdImpl(disambiguator, name); } @@ -37,32 +28,43 @@ final class BBIdImpl implements BBId { private final String name; private final String toString; + /** + * Returns the given ID casted. + * + *

This is safe because {@link BB} is sealed and {@link BBIdImpl} is the only permitted + * subclass. + */ + static BBIdImpl cast(BBId id) { + return switch (id) { + case BBIdImpl bbId -> bbId; + }; + } + BBIdImpl(int disambiguator, String name) { this.disambiguator = disambiguator; - this.name = NodeAndBBIds.quoteNameIfNecessary(name); - this.toString = "^" + disambiguator + name; + this.name = name; + this.toString = + "^" + (disambiguator == 0 ? "" : disambiguator) + NodeAndBBIds.quoteNameIfNecessary(name); } - /** Should only be used by {@link CFG}. */ int disambiguator() { return disambiguator; } - @Override - public String name() { + String name() { return name; } @Override public boolean equals(Object o) { if (this == o) return true; - if (!(o instanceof BBIdImpl bbId)) return false; - return disambiguator == bbId.disambiguator; + if (!(o instanceof BBIdImpl that)) return false; + return disambiguator == that.disambiguator && name.equals(that.name); } @Override public int hashCode() { - return Objects.hash(disambiguator); + return Objects.hash(disambiguator == 0 ? name : disambiguator); } @Override diff --git a/src/main/java/org/prlprg/ir/cfg/BBInline.java b/src/main/java/org/prlprg/ir/cfg/BBInline.java index 69a5722b8..4fd5f3fa4 100644 --- a/src/main/java/org/prlprg/ir/cfg/BBInline.java +++ b/src/main/java/org/prlprg/ir/cfg/BBInline.java @@ -33,7 +33,9 @@ default BB inlineAt(int index, CFG cfgToInline) { // Add BBs. var oldToNewBBs = cfgToInline.bbIds().stream() - .collect(Collectors.toMap(Function.identity(), id -> cfg().addBB(id.name()))); + .collect( + Collectors.toMap( + Function.identity(), id -> cfg().addBB(BBIdImpl.cast(id).name()))); // Add instructions, replace BB arguments, and prepare to replace node arguments. var newInstrOrPhis = new ArrayList(cfgToInline.numNodes()); @@ -69,13 +71,17 @@ default BB inlineAt(int index, CFG cfgToInline) { } for (var oldStmt : oldBB.stmts()) { var newStmt = - newBB.insertAt(newBB.stmts().size(), oldStmt.id().name(), oldStmt.data()); + newBB.insertAt( + newBB.stmts().size(), + InstrOrPhiIdImpl.cast(oldStmt.id()).name(), + oldStmt.data()); prePatchInstrOrPhi.accept(oldStmt, newStmt); } if (oldBB.jump() != null) { var newJump = newBB.addJump( - oldBB.jump().id().name(), oldBB.jump().data().replaceReturnWith(lastBB)); + InstrOrPhiIdImpl.cast(oldBB.jump().id()).name(), + oldBB.jump().data().replaceReturnWith(lastBB)); for (var oldTarget : newJump.targets()) { var newTarget = Objects.requireNonNull(oldToNewBBs.get(oldTarget.id())); newJump.replaceInTargets(oldTarget, newTarget); diff --git a/src/main/java/org/prlprg/ir/cfg/CFG.java b/src/main/java/org/prlprg/ir/cfg/CFG.java index dd6be5c0b..e427dcb40 100644 --- a/src/main/java/org/prlprg/ir/cfg/CFG.java +++ b/src/main/java/org/prlprg/ir/cfg/CFG.java @@ -47,16 +47,17 @@ public class CFG private final Set exits = new HashSet<>(); private final Map bbs = new HashMap<>(); private final Map, Node> nodes = new HashMap<>(); - // Disambiguator 1 is used for the entry. - private int nextBbDisambiguator = 2; - private int nextInstrOrPhiDisambiguator = 1; + private final NodeOrBBIdDisambiguatorMap nextBbDisambiguator = new NodeOrBBIdDisambiguatorMap(); + private final NodeOrBBIdDisambiguatorMap nextInstrOrPhiDisambiguator = + new NodeOrBBIdDisambiguatorMap(); private @Nullable DomTree cachedDomTree; private @Nullable DefUseAnalysis cachedDefUseAnalysis; /** Create a new CFG, with a single basic block and no instructions. */ @SuppressFBWarnings("CT_CONSTRUCTOR_THROW") public CFG() { - entry = new BB(this, new BBIdImpl(1, "entry")); + entry = new BB(this, new BBIdImpl(0, "entry")); + nextBbDisambiguator.add("entry", 0); bbs.put(entry.id(), entry); markExit(entry); } @@ -193,7 +194,7 @@ public BB addBB() { @Override public BB addBB(String name) { - return addBBWithId(new BBIdImpl(nextBbDisambiguator, name)); + return addBBWithId(uniqueBBId(name)); } @Override @@ -436,6 +437,8 @@ void record(Semantic edit, Semantic inverse) { } } + // region tracking and untracking + // region BBs /** * Insert and return a new basic block without recording it. * @@ -443,15 +446,18 @@ void record(Semantic edit, Semantic inverse) { * edits. */ BB doAddBB(String name) { - return doAddBBWithId(new BBIdImpl(nextBbDisambiguator, name)); + return doAddBBWithId(uniqueBBId(name)); } private BB doAddBBWithId(BBId id) { var bb = new BB(this, id); assert !bbs.containsKey(bb.id()); + bbs.put(id, bb); + markExit(bb); - updateNextBBDisambiguator(id); + var id1 = BBIdImpl.cast(id); + nextBbDisambiguator.add(id1.name(), id1.disambiguator()); return bb; } @@ -472,6 +478,8 @@ void doRemove(BB bb) { if (bb.successors().isEmpty()) { unmarkExit(bb); } + var id = BBIdImpl.cast(bb.id()); + nextBbDisambiguator.remove(id.name(), id.disambiguator()); } /** @@ -494,6 +502,9 @@ void unmarkExit(BB bb) { exits.remove(bb); } + // endregion BBs + + // region nodes /** * Mark an instruction or phi and its auxillary values as belonging to this CFG. * @@ -502,6 +513,10 @@ void unmarkExit(BB bb) { */ void track(InstrOrPhi instrOrPhi) { track((Node) instrOrPhi); + + var id = InstrOrPhiIdImpl.cast(instrOrPhi.id()); + nextInstrOrPhiDisambiguator.add(id.name(), id.disambiguator()); + if (instrOrPhi instanceof Instr instr) { for (var aux : instr.returns()) { if (aux != instrOrPhi) { @@ -532,6 +547,10 @@ void trackAll(Collection instrOrPhis) { */ void untrack(InstrOrPhi instrOrPhi) { untrack((Node) instrOrPhi); + + var id = InstrOrPhiIdImpl.cast(instrOrPhi.id()); + nextInstrOrPhiDisambiguator.remove(id.name(), id.disambiguator()); + if (instrOrPhi instanceof Instr instr) { for (var aux : instr.returns()) { if (aux != instrOrPhi) { @@ -583,20 +602,33 @@ private void untrack(Node node) { assert removed == node; } - /** Get the disambiguator and increment it. */ - int nextInstrOrPhiDisambiguator() { - return nextInstrOrPhiDisambiguator++; + // endregion nodes + // endregion tracking and untracking + + // endregion unique ids + private BBId uniqueBBId(String name) { + return new BBIdImpl(nextBbDisambiguator.get(name), name); } - /** Ensure that the next disambiguator is higher than the id's. */ - void updateNextInstrOrPhiDisambiguator(NodeId id) { - nextInstrOrPhiDisambiguator = - Math.max(nextInstrOrPhiDisambiguator, ((LocalNodeIdImpl) id).disambiguator() + 1); + /** + * Return a unique id for an {@linkplain InstrOrPhi instruction or phi} with no name. + * + *

The returned ID can be assigned to any type of instruction or phi, because its type checked + * at runtime is assigned in the {@link InstrOrPhiImpl} constructor. + */ + NodeId uniqueInstrOrPhiId() { + return this.uniqueInstrOrPhiId(""); } - /** Ensure that the next disambiguator is higher than the id's. */ - private void updateNextBBDisambiguator(BBId bbId) { - nextBbDisambiguator = Math.max(nextBbDisambiguator, ((BBIdImpl) bbId).disambiguator() + 1); + /** + * Return a unique id for an {@linkplain InstrOrPhi instruction or phi} with the given name. + * + *

The returned ID can be assigned to any type of instruction or phi, because its type checked + * at runtime is assigned in the {@link InstrOrPhiImpl} constructor. + */ + NodeId uniqueInstrOrPhiId(String name) { + return new InstrOrPhiIdImpl<>(nextInstrOrPhiDisambiguator.get(name), name); } + // region unique ids // endregion for BB and node } diff --git a/src/main/java/org/prlprg/ir/cfg/CFGEdit.java b/src/main/java/org/prlprg/ir/cfg/CFGEdit.java index 79e1614c1..80783f39f 100644 --- a/src/main/java/org/prlprg/ir/cfg/CFGEdit.java +++ b/src/main/java/org/prlprg/ir/cfg/CFGEdit.java @@ -79,10 +79,22 @@ sealed interface OnCFG> extends Semantic {} * instruction. */ sealed interface OnBB> extends Semantic { + /** Id of the {@linkplain BB} that the edit will affect. */ BBId bbId(); + /** + * Number of nodes affected by the edit. + * + *

If the edit only mutates nodes, {@link #sizeDelta()} will be 0 but this will be positive. + */ int numAffected(); + /** + * Number of nodes inserted (if positive) or removed (if negative) by the edit. + * + *

If the edit only mutates nodes, this will be 0 but {@link #numAffected()} will be + * positive. + */ int sizeDelta(); } @@ -93,6 +105,7 @@ sealed interface OnBB> extends Semantic { * (excluded from the definition of "local" used within this, {@link OnCFG}, and {@link OnBB}). */ sealed interface OnInstrOrPhi> extends Semantic { + /** Id of the {@linkplain Node node} that the edit will affect. */ NodeId targetId(); } @@ -160,11 +173,10 @@ public InsertBB(BB bb) { public RemoveBB apply(CFG cfg) { var bb = cfg.addBBWithId(id); bb.addPhisWithIds(phiArgs.stream().map(a -> new PhiImpl.Args(cfg, a)).toList()); - bb.insertAllAtWithTokens( + bb.insertAllAtWithIds( 0, stmtIdsAndArgs.stream().map(a -> new StmtImpl.Args(cfg, a)).toList()); if (jumpIdAndArgs != null) { - bb.addJumpWithToken( - new CreateInstrWithExistingId(jumpIdAndArgs.id()), jumpIdAndArgs.data().decode(cfg)); + bb.addJumpWithId(jumpIdAndArgs.id(), jumpIdAndArgs.data().decode(cfg)); } return new RemoveBB(bb); @@ -288,7 +300,7 @@ public int numAffected() { public RemoveStmts apply(CFG cfg) { var stmts = cfg.get(bbId) - .insertAllAtWithTokens( + .insertAllAtWithIds( index, this.stmts.stream().map(a -> new StmtImpl.Args(cfg, a)).toList()); return new RemoveStmts(bbId, index, index + stmts.size()); } @@ -320,7 +332,7 @@ public int numAffected() { public RemoveStmts2 apply(CFG cfg) { var indicesAndStmts = cfg.get(bbId) - .insertAllAtWithTokens( + .insertAllAtWithIds( this.indicesAndStmts.entrySet().stream() .collect( Collectors.toMap( @@ -365,7 +377,7 @@ public int numAffected() { @Override public RemoveStmt apply(CFG cfg) { - cfg.get(bbId).insertAtWithToken(index, new CreateInstrWithExistingId(id), args.decode(cfg)); + cfg.get(bbId).insertAtWithId(index, id, args.decode(cfg)); return new RemoveStmt(bbId, index); } } @@ -390,7 +402,7 @@ public int numAffected() { @Override public RemoveJump apply(CFG cfg) { - cfg.get(bbId).addJumpWithToken(new CreateInstrWithExistingId(nodeId), args.decode(cfg)); + cfg.get(bbId).addJumpWithId(nodeId, args.decode(cfg)); return new RemoveJump(bbId); } } @@ -550,14 +562,26 @@ public SetPhiInput apply(CFG cfg) { } } - record RenameInstrOrPhi(NodeId targetId, String newName) - implements MutateInstrOrPhi { + record MutateInstrOrPhiId(NodeId oldId, NodeId newId) + implements MutateInstrOrPhi { + public MutateInstrOrPhiId { + if (oldId.clazz() != newId.clazz()) { + throw new IllegalArgumentException( + "`MutateInstrOrPhiId` must preserve the node ID's class: the node's class doesn't change, so the ID class shouldn't either"); + } + } + + /** Alias for {@code oldId} for this to implement {@link MutateInstrOrPhi#targetId()}. */ @Override - public RenameInstrOrPhi apply(CFG cfg) { - var target = cfg.get(targetId); - var oldName = targetId.name(); - target.rename(newName); - return new RenameInstrOrPhi(targetId, oldName); + public NodeId targetId() { + return oldId; + } + + @Override + public MutateInstrOrPhiId apply(CFG cfg) { + var old = cfg.get(oldId); + InstrOrPhiImpl.cast(old).setId(newId); + return new MutateInstrOrPhiId(newId, oldId); } } @@ -589,14 +613,14 @@ public ReplaceStmt(BB bb, int index, String newName, StmtData newAr @SuppressWarnings("unchecked") public ReplaceStmt(BB bb, int index, I stmt) { - this(bb, index, stmt.id().name(), (StmtData) stmt.data()); + this(bb, index, InstrOrPhiIdImpl.cast(stmt.id()).name(), (StmtData) stmt.data()); } @Override public ReplaceStmt apply(CFG cfg) { var bb = cfg.get(bbId); var old = bb.stmt(index); - var oldName = old.id().name(); + var oldName = InstrOrPhiIdImpl.cast(old.id()).name(); // `replaceNoSubst` *does not* require that `oldData` has the same erased type as `newData`; // you can replace an instruction with one of a different type. This is why the reverse action // has an erased `I`. @@ -617,14 +641,14 @@ public ReplaceJump(BB bb, String newName, JumpData newArgs) { @SuppressWarnings("unchecked") public ReplaceJump(BB bb, I jump) { - this(bb, jump.id().name(), (JumpData) jump.data()); + this(bb, InstrOrPhiIdImpl.cast(jump.id()).name(), (JumpData) jump.data()); } @Override public ReplaceJump apply(CFG cfg) { var bb = cfg.get(bbId); var old = bb.jump(); - var oldName = old == null ? null : old.id().name(); + var oldName = old == null ? null : InstrOrPhiIdImpl.cast(old.id()).name(); var oldArgs = old == null ? null : MapToIdIn.of(old.data()); bb.replaceJump(newName, newArgs.decode(cfg)); // Throws if `old == null` assert oldName != null; // implies `oldArgs == null` diff --git a/src/main/java/org/prlprg/ir/cfg/CFGParsePrint.java b/src/main/java/org/prlprg/ir/cfg/CFGParsePrint.java index 9f1db706b..a519884b8 100644 --- a/src/main/java/org/prlprg/ir/cfg/CFGParsePrint.java +++ b/src/main/java/org/prlprg/ir/cfg/CFGParsePrint.java @@ -229,7 +229,8 @@ private Phi parsePhi(Parser p) { } var s = p.scanner(); - // Cast is OK because we parsed, so it has no assigned class. + // A newly-created instruction or phi ID's type parameter is allowed to be anything until it + // gets passed to the `InstrOrPhiImpl` constructor, so the unchecked cast is safe. @SuppressWarnings("unchecked") var id = (NodeId>) p.parse(NodeId.class); ensureIdNotTakenByAnonymous(id, p); @@ -298,15 +299,14 @@ private Instr parseInstr(Parser p) { } var s = p.scanner(); - TokenToCreateNewInstr token; + // A newly-created instruction or phi ID's type parameter is allowed to be anything until it + // gets passed to the `InstrOrPhiImpl` constructor, so the following unchecked casts are safe. + NodeId id = null; if (s.nextCharIs('%')) { @SuppressWarnings("unchecked") - var id = (NodeId) p.parse(NodeId.class); - token = new CreateInstrWithExistingId(id); + var id1 = (NodeId) p.parse(NodeId.class); + id = id1; s.assertAndSkip('='); - } else { - // ID doesn't matter - token = new CreateInstrWithNewId(""); } var data = p.withContext(dataContext).parse(InstrData.class); @@ -314,14 +314,25 @@ private Instr parseInstr(Parser p) { // This must go after `parse(InstrData.class)`, because the latter may re-assign some `Void` // instruction IDs. If it went before, one of those re-assigned IDs may have been assigned to // `id`, so it would no longer be free. - if (token instanceof CreateInstrWithExistingId(var id)) { + if (id == null) { + // ID doesn't matter + id = cfg.uniqueInstrOrPhiId(); + } else { ensureIdNotTakenByAnonymous(id, p); } var instr = switch (data) { - case StmtData stmtData -> bb.insertAtWithToken(bb.stmts().size(), token, stmtData); - case JumpData jumpData -> bb.addJumpWithToken(token, jumpData); + case StmtData stmtData -> { + @SuppressWarnings("unchecked") + var id1 = (NodeId) id; + yield bb.insertAtWithId(bb.stmts().size(), id1, stmtData); + } + case JumpData jumpData -> { + @SuppressWarnings("unchecked") + var id1 = (NodeId) id; + yield bb.addJumpWithId(id1, jumpData); + } }; patchIfPending(instr); @@ -352,8 +363,9 @@ private void ensureIdNotTakenByAnonymous(NodeId id, Parser if (!old.returns().isEmpty()) { throw s.fail("Defined the same ID twice: " + id); } - assert id.name().isEmpty() : "expected ID of `Void` instruction we created to have no name"; - old.updateDisambiguator(); + assert InstrOrPhiIdImpl.cast(id).name().isEmpty() + : "expected ID of `Void` instruction we created to have no name"; + InstrOrPhiImpl.cast(old).setId(cfg.uniqueInstrOrPhiId()); } } @@ -424,7 +436,9 @@ private Node parseNode(Parser p, Class nodeClass) { if (!(id instanceof GlobalNodeId) && cfg.contains(id)) { var node = cfg.get(id); if (node instanceof Instr i && i.returns().isEmpty()) { - i.updateDisambiguator(); + assert InstrOrPhiIdImpl.cast(i.id()).name().isEmpty() + : "expected ID of `Void` instruction we created to have no name"; + InstrOrPhiImpl.cast(i).setId(cfg.uniqueInstrOrPhiId()); // `cfg.contains(id)` is now `false`. } } diff --git a/src/main/java/org/prlprg/ir/cfg/CFGPirSerialize.java b/src/main/java/org/prlprg/ir/cfg/CFGPirSerialize.java index 09ed02a13..6d26d7751 100644 --- a/src/main/java/org/prlprg/ir/cfg/CFGPirSerialize.java +++ b/src/main/java/org/prlprg/ir/cfg/CFGPirSerialize.java @@ -16,6 +16,7 @@ import java.util.stream.Stream; import javax.annotation.Nullable; import org.prlprg.bc.Bc; +import org.prlprg.ir.cfg.JumpData.Checkpoint_; import org.prlprg.ir.cfg.PirType.NativeType; import org.prlprg.ir.cfg.PirType.RType_; import org.prlprg.ir.cfg.StmtData.Call_; @@ -614,7 +615,7 @@ private InstrOrPhi parseInstrOrPhi(Parser p, String instrTypeName, PirType pirTy var failBB = bbs.get(failBBIndex); s.assertAndSkip(" (if assume failed)"); - yield new JumpData.Checkpoint(ImmutableList.of(), ImmutableList.of(), passBB, failBB); + yield new Checkpoint_(ImmutableList.of(), ImmutableList.of(), passBB, failBB); } case "Deopt" -> { var frameState = p.parse(FrameState.class); @@ -1406,7 +1407,7 @@ void printArgs(InstrOrPhi self, Printer p) { p.print(bbToIdx.get(b.ifFalse())); w.write(" (if false)"); } - case JumpData.Checkpoint c -> { + case JumpData.Checkpoint_ c -> { // REACH: Print assumptions (tests and fail reasons) separately w.write("-> BB"); p.print(bbToIdx.get(c.ifPass())); @@ -1697,7 +1698,7 @@ private static final class GlobalNamed extends PirId { private final Node node; GlobalNamed(Node node) { - this(node, node.id().name()); + this(node, node.id() instanceof InstrOrPhiIdImpl id ? id.name() : node.id().toString()); } GlobalNamed(Node node, String id) { diff --git a/src/main/java/org/prlprg/ir/cfg/Call.java b/src/main/java/org/prlprg/ir/cfg/Call.java index c7a25f646..cc0ffe768 100644 --- a/src/main/java/org/prlprg/ir/cfg/Call.java +++ b/src/main/java/org/prlprg/ir/cfg/Call.java @@ -20,8 +20,8 @@ final class CallImpl extends AbstractRValueStmtImpl> implements Call { // Idk how Java generics work, is there a potential issue from casting `Class` to // `Class>`? @SuppressWarnings("unchecked") - CallImpl(CFG cfg, TokenToCreateNewInstr token, Call_ data) { - super((Class>) (Object) Call_.class, cfg, token, data); + CallImpl(CFG cfg, NodeId id, Call_ data) { + super((Class>) (Object) Call_.class, cfg, id, data); } @Override diff --git a/src/main/java/org/prlprg/ir/cfg/Checkpoint.java b/src/main/java/org/prlprg/ir/cfg/Checkpoint.java index 63af43fa6..e8755c65d 100644 --- a/src/main/java/org/prlprg/ir/cfg/Checkpoint.java +++ b/src/main/java/org/prlprg/ir/cfg/Checkpoint.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.function.Predicate; import java.util.stream.Stream; +import org.prlprg.ir.cfg.JumpData.Checkpoint_; import org.prlprg.util.Pair; /** @@ -18,12 +19,12 @@ public interface Checkpoint extends Jump { NodeId id(); @Override - JumpData.Checkpoint data(); + Checkpoint_ data(); default void addAssumption(RValue test, org.prlprg.rshruntime.DeoptReason failReason) { Instr.mutateArgs( this, - new JumpData.Checkpoint( + new Checkpoint_( Stream.concat(data().tests().stream(), Stream.of(test)) .collect(ImmutableList.toImmutableList()), Stream.concat(data().failReasons().stream(), Stream.of(failReason)) @@ -38,7 +39,7 @@ default void filterAssumptions(Predicate keepTest) { data().streamAssumptionData().filter(pair -> keepTest.test(pair.first())).toList(); Instr.mutateArgs( this, - new JumpData.Checkpoint( + new Checkpoint_( testsAndFailReasons.stream().map(Pair::first).collect(ImmutableList.toImmutableList()), testsAndFailReasons.stream().map(Pair::second).collect(ImmutableList.toImmutableList()), data().ifPass(), @@ -46,11 +47,11 @@ default void filterAssumptions(Predicate keepTest) { } } -final class CheckpointImpl extends JumpImpl implements Checkpoint { +final class CheckpointImpl extends JumpImpl implements Checkpoint { private final List assumptions = new ArrayList<>(); - CheckpointImpl(CFG cfg, TokenToCreateNewInstr token, JumpData.Checkpoint data) { - super(JumpData.Checkpoint.class, cfg, token, data); + CheckpointImpl(CFG cfg, NodeId id, Checkpoint_ data) { + super(Checkpoint_.class, cfg, id, data); // This isn't handled by `verify` because at the time of the call, `assumptions` is still null. while (assumptions.size() < data.numAssumptions()) { diff --git a/src/main/java/org/prlprg/ir/cfg/DeoptReasonPhi.java b/src/main/java/org/prlprg/ir/cfg/DeoptReasonPhi.java index 555b201e6..4b748d8d8 100644 --- a/src/main/java/org/prlprg/ir/cfg/DeoptReasonPhi.java +++ b/src/main/java/org/prlprg/ir/cfg/DeoptReasonPhi.java @@ -1,7 +1,6 @@ package org.prlprg.ir.cfg; import java.util.Collection; -import javax.annotation.Nullable; /** * {@link Phi} (, DeoptReason { } final class DeoptReasonPhiImpl extends PhiImpl implements DeoptReasonPhi { - DeoptReasonPhiImpl(CFG cfg, @Nullable NodeId presetId, Collection> inputs) { - super(DeoptReason.class, cfg, presetId, inputs); + DeoptReasonPhiImpl(CFG cfg, NodeId> id, Collection> inputs) { + super(DeoptReason.class, cfg, id, inputs); } @Override diff --git a/src/main/java/org/prlprg/ir/cfg/FrameStatePhi.java b/src/main/java/org/prlprg/ir/cfg/FrameStatePhi.java index 807511d17..6f81780a5 100644 --- a/src/main/java/org/prlprg/ir/cfg/FrameStatePhi.java +++ b/src/main/java/org/prlprg/ir/cfg/FrameStatePhi.java @@ -1,7 +1,6 @@ package org.prlprg.ir.cfg; import java.util.Collection; -import javax.annotation.Nullable; /** * {@link Phi} (, FrameState { } final class FrameStatePhiImpl extends PhiImpl implements FrameStatePhi { - FrameStatePhiImpl(CFG cfg, @Nullable NodeId presetId, Collection> inputs) { - super(FrameState.class, cfg, presetId, inputs); + FrameStatePhiImpl(CFG cfg, NodeId> id, Collection> inputs) { + super(FrameState.class, cfg, id, inputs); } @Override diff --git a/src/main/java/org/prlprg/ir/cfg/FrameStateStmt.java b/src/main/java/org/prlprg/ir/cfg/FrameStateStmt.java index 7b74ad4c0..a8d2a10f4 100644 --- a/src/main/java/org/prlprg/ir/cfg/FrameStateStmt.java +++ b/src/main/java/org/prlprg/ir/cfg/FrameStateStmt.java @@ -15,8 +15,8 @@ public interface FrameStateStmt extends Stmt, org.prlprg.ir.cfg.FrameState { } final class FrameStateStmtImpl extends SelfReturningStmtImpl implements FrameStateStmt { - FrameStateStmtImpl(CFG cfg, TokenToCreateNewInstr token, FrameState data) { - super(FrameState.class, cfg, token, data); + FrameStateStmtImpl(CFG cfg, NodeId id, FrameState data) { + super(FrameState.class, cfg, id, data); } @Override diff --git a/src/main/java/org/prlprg/ir/cfg/GlobalNodeId.java b/src/main/java/org/prlprg/ir/cfg/GlobalNodeId.java index 2748b2a61..834cfc0da 100644 --- a/src/main/java/org/prlprg/ir/cfg/GlobalNodeId.java +++ b/src/main/java/org/prlprg/ir/cfg/GlobalNodeId.java @@ -80,11 +80,6 @@ public N node() { return node; } - @Override - public String name() { - return Printer.use(this::printName); - } - @Override public String toString() { return Printer.toString(this); @@ -133,12 +128,6 @@ private void print(Printer p) { w.write(nodeTypeName(node)); w.write('\\'); - printName(p); - } - - private void printName(Printer p) { - var w = p.writer(); - switch (node) { case Constant(var constant) -> p.print(constant); case ConstantDeoptReason(var dr) -> p.print(dr); diff --git a/src/main/java/org/prlprg/ir/cfg/Instr.java b/src/main/java/org/prlprg/ir/cfg/Instr.java index 28c331fa5..06cf1fd5d 100644 --- a/src/main/java/org/prlprg/ir/cfg/Instr.java +++ b/src/main/java/org/prlprg/ir/cfg/Instr.java @@ -13,15 +13,10 @@ import org.checkerframework.checker.index.qual.SameLen; import org.jetbrains.annotations.UnmodifiableView; import org.prlprg.ir.cfg.CFGEdit.MutateInstrArgs; -import org.prlprg.ir.cfg.CFGEdit.RenameInstrOrPhi; import org.prlprg.ir.closure.CodeObject; import org.prlprg.ir.type.REffects; import org.prlprg.ir.type.RType; import org.prlprg.ir.type.RTypes; -import org.prlprg.parseprint.ParseMethod; -import org.prlprg.parseprint.Parser; -import org.prlprg.parseprint.PrintMethod; -import org.prlprg.parseprint.Printer; import org.prlprg.sexp.SEXPType; import org.prlprg.util.Classes; import org.prlprg.util.Reflection; @@ -57,7 +52,7 @@ public sealed interface Instr extends InstrOrPhi permits Jump, Stmt { * still need to cast the {@code args}, fortunately the compatibility is also checked at runtime. */ static void mutateArgs(I instr, InstrData newArgs) { - if (!((InstrImpl) instr).canReplaceDataWith(newArgs)) { + if (!InstrImpl.cast(instr).canReplaceDataWith(newArgs)) { throw new IllegalArgumentException( "incompatible data for replacement: " + instr + " -> " + newArgs); } @@ -72,7 +67,7 @@ static void mutateArgs(I instr, InstrData newArgs) { @SuppressWarnings("unchecked") var oldArgs = (InstrData) instr.data(); - ((InstrImpl) instr).unsafeReplaceArgs(newArgs); + InstrImpl.cast(instr).unsafeReplaceArgs(newArgs); if (instr instanceof Jump j) { var bb = ((JumpImpl) j).bb(); @@ -149,11 +144,9 @@ default boolean isPure() { NodeId id(); } -abstract sealed class InstrImpl> implements LocalNode +abstract sealed class InstrImpl> extends InstrOrPhiImpl implements LocalNode permits JumpImpl, StmtImpl { private final Class dataClass; - private final CFG cfg; - private LocalNodeIdImpl id; private D data; // These are both initialized in `verify`, which is called from the constructor, and doesn't @@ -164,28 +157,78 @@ abstract sealed class InstrImpl> implements LocalNode @SuppressWarnings("NotNullFieldNotInitialized") private REffects effects; - InstrImpl(Class dataClass, CFG cfg, TokenToCreateNewInstr token, D data) { + /** + * Return the given instruction casted. + * + *

Any {@link Instr} is guaranteed to be an {@link InstrImpl}, so this method is provided to + * reduce the number of casts in the code text. + */ + static InstrImpl cast(Instr instr) { + return (InstrImpl) instr; + } + + InstrImpl(Class dataClass, CFG cfg, NodeId id, D data) { + super(cfg, id); this.dataClass = dataClass; - this.cfg = cfg; - id = - switch (token) { - case CreateInstrWithExistingId(var id1) -> { - var id2 = (LocalNodeIdImpl) id1; - id2.lateAssignClass(getClass()); - cfg.updateNextInstrOrPhiDisambiguator(id2); - yield id2; - } - case CreateInstrWithNewId(var name) -> new LocalNodeIdImpl<>((Instr) this, name); - }; this.data = data; verify(true); } + // region data + // @Override + public final D data() { + return data; + } + + /** + * Whether the instruction's data can be replaced with the specified instance. Calling {@link + * Instr#mutateArgs(Instr, InstrData)} will not throw iff this returns {@code true}. + */ + public final boolean canReplaceDataWith(InstrData newData) { + return dataClass.isInstance(newData); + } + + /** + * Replace the instruction's data without updating BB predecessors or recording an + * {@linkplain CFGEdit edit}. + * + *

Call {@link Instr#mutateArgs(Instr, InstrData)} to update predecessors and record an edit. + * + * @throws IllegalArgumentException If the new type of data isn't compatible with the instruction. + */ + @SuppressWarnings("unchecked") + public final void unsafeReplaceArgs(InstrData newData) { + if (!canReplaceDataWith(newData)) { + throw new IllegalArgumentException( + "Can't replace data in " + + getClass().getSimpleName() + + " with incompatible type " + + newData.getClass().getSimpleName()); + } + + data = (D) newData; + } + + // endregion data + + // region args // @Override public final ImmutableList args() { return args; } + // @Override + @SuppressWarnings("unchecked") + public final void replaceInArgs(Node old, Node replacement) { + var oldData = data; + unsafeReplaceInData("node", old, replacement); + + cfg() + .record( + new CFGEdit.MutateInstrArgs<>((Instr) this, (InstrData) data), + new CFGEdit.MutateInstrArgs<>((Instr) this, (InstrData) oldData)); + } + private void computeArgs() { // Reflectively get all Node record components if (!(data instanceof Record r)) { @@ -340,39 +383,9 @@ protected final void unsafeReplaceInData(String argTypeStr, Object old, Object r verify(); } - @SuppressWarnings("unchecked") - public final void replaceInArgs(Node old, Node replacement) { - var oldData = data; - unsafeReplaceInData("node", old, replacement); - - cfg.record( - new CFGEdit.MutateInstrArgs<>((Instr) this, (InstrData) data), - new CFGEdit.MutateInstrArgs<>((Instr) this, (InstrData) oldData)); - } - - /** - * If this is an {@code Optional}, {@code ImmutableList}, or {@code - * ImmutableList>}, returns {@code T}. - */ - private static Class optionalOrCollectionComponentElementClass(RecordComponent cmp) { - assert cmp.getType() == Optional.class || cmp.getType() == ImmutableList.class - : "`InstrData` has `Collection` component which isn't an `ImmutableList`"; - if (cmp.getGenericType() instanceof ParameterizedType p - && p.getActualTypeArguments().length == 1) { - if (p.getActualTypeArguments()[0] instanceof ParameterizedType innerP - && innerP.getRawType() == Optional.class - && innerP.getActualTypeArguments().length == 1 - && innerP.getActualTypeArguments()[0] instanceof Class elemClass) { - return elemClass; - } else if (p.getActualTypeArguments()[0] instanceof Class elemClass) { - return elemClass; - } - } - throw new AssertionError( - "`InstrData` has `Collection` component which isn't a straightforward generic type, don't know how to handle: " - + cmp.getGenericType()); - } + // endregion args + // region effects // @Override public final REffects effects() { return effects; @@ -427,55 +440,9 @@ protected T mergeComputed(String desc, @Nullable T fromOverride, @Nullable T return fromOverride != null ? fromOverride : fromAnnotation; } - @Override - public final CFG cfg() { - return cfg; - } - - // @Override - public final void rename(String newName) { - var oldId = id(); - - cfg().untrack((Instr) this); - id = new LocalNodeIdImpl<>((Instr) this, newName); - cfg().track((Instr) this); - - cfg().record(new RenameInstrOrPhi(oldId, newName), new RenameInstrOrPhi(id(), oldId.name())); - } - - public final D data() { - return data; - } - - /** - * Whether the instruction's data can be replaced with the specified instance. Calling {@link - * Instr#mutateArgs(Instr, InstrData)} will not throw iff this returns {@code true}. - */ - public final boolean canReplaceDataWith(InstrData newData) { - return dataClass.isInstance(newData); - } - - /** - * Replace the instruction's data without updating BB predecessors or recording an - * {@linkplain CFGEdit edit}. - * - *

Call {@link Instr#mutateArgs(Instr, InstrData)} to update predecessors and record an edit. - * - * @throws IllegalArgumentException If the new type of data isn't compatible with the instruction. - */ - @SuppressWarnings("unchecked") - public final void unsafeReplaceArgs(InstrData newData) { - if (!canReplaceDataWith(newData)) { - throw new IllegalArgumentException( - "Can't replace data in " - + getClass().getSimpleName() - + " with incompatible type " - + newData.getClass().getSimpleName()); - } - - data = (D) newData; - } + // endregion effects + // region verify // @Override public final void verify() throws InstrVerifyException { verify(false); @@ -673,28 +640,35 @@ private void verifyRValueArgsAreOfCorrectTypes() throws InstrVerifyException { } } + // endregion verify + @Override public NodeId id() { return uncheckedCastId(); } - @SuppressWarnings("unchecked") - protected NodeId uncheckedCastId() { - return (NodeId) id; - } - - @Override - public String toString() { - return Printer.toString(this); - } - - @ParseMethod - private Instr parse(Parser p) { - throw new UnsupportedOperationException("can't parse a Phi outside of a BB"); - } - - @PrintMethod - private void print(Printer p) { - p.withContext(new CFGParseOrPrintContext(p.context(), cfg()).new BBContext(null)).print(this); + // region misc reflection helpers + /** + * If this is an {@code Optional}, {@code ImmutableList}, or {@code + * ImmutableList>}, returns {@code T}. + */ + private static Class optionalOrCollectionComponentElementClass(RecordComponent cmp) { + assert cmp.getType() == Optional.class || cmp.getType() == ImmutableList.class + : "`InstrData` has `Collection` component which isn't an `ImmutableList`"; + if (cmp.getGenericType() instanceof ParameterizedType p + && p.getActualTypeArguments().length == 1) { + if (p.getActualTypeArguments()[0] instanceof ParameterizedType innerP + && innerP.getRawType() == Optional.class + && innerP.getActualTypeArguments().length == 1 + && innerP.getActualTypeArguments()[0] instanceof Class elemClass) { + return elemClass; + } else if (p.getActualTypeArguments()[0] instanceof Class elemClass) { + return elemClass; + } + } + throw new AssertionError( + "`InstrData` has `Collection` component which isn't a straightforward generic type, don't know how to handle: " + + cmp.getGenericType()); } + // endregion misc reflection helpers } diff --git a/src/main/java/org/prlprg/ir/cfg/InstrData.java b/src/main/java/org/prlprg/ir/cfg/InstrData.java index 17e7ca125..0145f4d00 100644 --- a/src/main/java/org/prlprg/ir/cfg/InstrData.java +++ b/src/main/java/org/prlprg/ir/cfg/InstrData.java @@ -27,10 +27,10 @@ public sealed interface InstrData permits JumpData, StmtData { /** * Create an instruction containing this data. * - *

This can only be called in the package due to {@link TokenToCreateNewInstr} instances being - * package-private and never exposed by its API. + *

This should only be called in the package, since the returned value will not be tracked in + * the {@link CFG}, attempting to insert it will raise errors. */ - I make(CFG cfg, TokenToCreateNewInstr token); + I make(CFG cfg, NodeId id); /** * Compute the effects for this instruction, or for trivial cases, this will return {@code diff --git a/src/main/java/org/prlprg/ir/cfg/InstrOrPhi.java b/src/main/java/org/prlprg/ir/cfg/InstrOrPhi.java index eba58d3f8..42bf1bde1 100644 --- a/src/main/java/org/prlprg/ir/cfg/InstrOrPhi.java +++ b/src/main/java/org/prlprg/ir/cfg/InstrOrPhi.java @@ -25,22 +25,9 @@ public sealed interface InstrOrPhi extends LocalNode permits Instr, Phi { /** Whether the instruction or phi produces no side-effects (for phis this is always true). */ boolean isPure(); - /** - * Change the instruction or phi's name (descriptive identifier). - * - *

Even if called with the same name, it will change the ID's disambiguator. - */ + /** Change the instruction or phi's name (descriptive identifier). */ void rename(String newName); - /** - * Change the instruction or phi's disambiguator. - * - *

This just calls {@link #rename(String)} with the current name. - */ - default void updateDisambiguator() { - rename(id().name()); - } - /** * Replace {@code old} with {@code replacement} in {@link #args()} and (if instruction) {@link * Instr#data()}. diff --git a/src/main/java/org/prlprg/ir/cfg/InstrOrPhiImpl.java b/src/main/java/org/prlprg/ir/cfg/InstrOrPhiImpl.java new file mode 100644 index 000000000..c11f9fd1d --- /dev/null +++ b/src/main/java/org/prlprg/ir/cfg/InstrOrPhiImpl.java @@ -0,0 +1,90 @@ +package org.prlprg.ir.cfg; + +import org.prlprg.ir.cfg.CFGEdit.MutateInstrOrPhiId; +import org.prlprg.parseprint.ParseMethod; +import org.prlprg.parseprint.Parser; +import org.prlprg.parseprint.PrintMethod; +import org.prlprg.parseprint.Printer; + +/** + * Common package-private logic between {@linkplain Instr instructions} and {@linkplain Phi phis}. + */ +abstract sealed class InstrOrPhiImpl implements LocalNode permits PhiImpl, InstrImpl { + private final CFG cfg; + private InstrOrPhiIdImpl id; + + /** + * Return the given instruction or phi casted. + * + *

Any {@link InstrOrPhi} is guaranteed to be an {@link InstrOrPhiImpl}, so this method is + * provided to reduce the number of casts in the code text. + */ + static InstrOrPhiImpl cast(InstrOrPhi instrOrPhi) { + return (InstrOrPhiImpl) instrOrPhi; + } + + protected InstrOrPhiImpl(CFG cfg, NodeId id) { + this.cfg = cfg; + this.id = InstrOrPhiIdImpl.cast(id); + this.id.lateAssignClass(getClass()); + } + + // @Override + public final void rename(String newName) { + setId(cfg().uniqueInstrOrPhiId(newName)); + } + + @Override + public NodeId id() { + return uncheckedCastId(); + } + + @Override + public CFG cfg() { + return cfg; + } + + @SuppressWarnings("unchecked") + protected NodeId uncheckedCastId() { + return (NodeId) id; + } + + /** + * Change the instruction or phi's id. + * + *

This is called internally and only when a specific ID is needed (e.g. for {@link CFGEdit} + * undo/replay or {@link CFGCleanup} re-numbering), otherwise call {@link #rename(String)}. + * + * @throws IllegalArgumentException If the ID class isn't delayed or the instruction or phi class. + * @throws IllegalArgumentException If the ID is taken by another instruction or phi in the CFG. + */ + final void setId(NodeId newId) { + var oldId = id(); + + var newId1 = InstrOrPhiIdImpl.cast(newId); + newId1.lateAssignClass(getClass()); + + cfg().untrack((InstrOrPhi) this); + id = newId1; + cfg().track((InstrOrPhi) this); + + cfg().record(new MutateInstrOrPhiId(oldId, newId), new MutateInstrOrPhiId(newId, oldId)); + } + + // region serialization and deserialization + @ParseMethod + private Phi parse(Parser p) { + throw new UnsupportedOperationException("can't parse an instruction or phi outside of a BB"); + } + + @PrintMethod + private void print(Printer p) { + p.withContext(new CFGParseOrPrintContext(p.context(), cfg()).new BBContext(null)).print(this); + } + + @Override + public String toString() { + return Printer.toString(this); + } + // endregion serialization and deserialization +} diff --git a/src/main/java/org/prlprg/ir/cfg/Jump.java b/src/main/java/org/prlprg/ir/cfg/Jump.java index ae0d6306a..63c2c05a1 100644 --- a/src/main/java/org/prlprg/ir/cfg/Jump.java +++ b/src/main/java/org/prlprg/ir/cfg/Jump.java @@ -39,8 +39,8 @@ record Serial(NodeId id, MapToIdIn> data) abstract non-sealed class JumpImpl> extends InstrImpl implements Jump { /** - * This is only {@code null} to keep {@link InstrData#make(CFG, TokenToCreateNewInstr)} not - * require {@link BB}. It gets set immediately after creation. + * This is only {@code null} to keep {@link InstrData#make(CFG, NodeId)} not require {@link BB}. + * It gets set immediately after creation. */ private @Nullable BB bb; @@ -49,8 +49,8 @@ abstract non-sealed class JumpImpl> extends InstrImpl i @SuppressFBWarnings("NP_NONNULL_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR") private ImmutableList targets; - JumpImpl(Class dataClass, CFG cfg, TokenToCreateNewInstr token, D data) { - super(dataClass, cfg, token, data); + JumpImpl(Class dataClass, CFG cfg, NodeId id, D data) { + super(dataClass, cfg, id, data); } /** @@ -132,8 +132,8 @@ public NodeId id() { /** {@link Jump} which doesn't return anything. */ final class VoidJumpImpl extends JumpImpl { - VoidJumpImpl(CFG cfg, TokenToCreateNewInstr token, JumpData.Void data) { - super(JumpData.Void.class, cfg, token, data); + VoidJumpImpl(CFG cfg, NodeId id, JumpData.Void data) { + super(JumpData.Void.class, cfg, id, data); } @Override diff --git a/src/main/java/org/prlprg/ir/cfg/JumpData.java b/src/main/java/org/prlprg/ir/cfg/JumpData.java index e26c44191..dbabd2602 100644 --- a/src/main/java/org/prlprg/ir/cfg/JumpData.java +++ b/src/main/java/org/prlprg/ir/cfg/JumpData.java @@ -23,7 +23,7 @@ default JumpData replaceReturnWith(BB newTarget) { sealed interface Void extends JumpData { @Override - default Jump make(CFG cfg, TokenToCreateNewInstr id) { + default Jump make(CFG cfg, NodeId id) { return new VoidJumpImpl(cfg, id, this); } } @@ -64,12 +64,12 @@ public JumpData replaceReturnWith(BB newTarget) { record Unreachable() implements Void {} @EffectsAre({}) - record Checkpoint( + record Checkpoint_( @TypeIs("BOOL") ImmutableList tests, @SameLen("tests") ImmutableList failReasons, BB ifPass, BB ifFail) - implements JumpData { + implements JumpData { public int numAssumptions() { return tests.size(); } @@ -80,7 +80,7 @@ public Stream> streamAssumptionD } @Override - public org.prlprg.ir.cfg.Checkpoint make(CFG cfg, TokenToCreateNewInstr id) { + public org.prlprg.ir.cfg.Checkpoint make(CFG cfg, NodeId id) { return new CheckpointImpl(cfg, id, this); } } diff --git a/src/main/java/org/prlprg/ir/cfg/NodeId.java b/src/main/java/org/prlprg/ir/cfg/NodeId.java index f2ab4591d..1d8ed4aa6 100644 --- a/src/main/java/org/prlprg/ir/cfg/NodeId.java +++ b/src/main/java/org/prlprg/ir/cfg/NodeId.java @@ -7,7 +7,6 @@ import org.prlprg.parseprint.Parser; import org.prlprg.parseprint.Scanner; import org.prlprg.parseprint.SkipWhitespace; -import org.prlprg.util.Classes; /** * Identifies a {@linkplain Node node} of class {@code N} within a {@linkplain CFG control-flow @@ -70,24 +69,12 @@ static boolean isNodeIdStart(Scanner s) { */ @Nullable Class clazz(); - /** Whether the ID is for a local or global node. */ - boolean isLocal(); - /** - * Descriptive name to make the logic in the CFG clearer, or empty string if there is none. - * - *

Specifically: + * Whether the ID is for a local or {@linkplain GlobalNode global node}. * - *

- * - *

Different IDs in the same CFG may have the same name; use {@link Object#toString()} to - * return a unique string representation. + *

This is {@code false} iff {@code this instanceof }{@link GlobalNodeId}. */ - String name(); + boolean isLocal(); @ParseMethod(SkipWhitespace.NONE) private static NodeId parse(Parser p) { @@ -119,42 +106,29 @@ protected NodeIdImpl(@Nullable Class clazz) { } } -final class LocalNodeIdImpl extends NodeIdImpl { - private final int disambiguator; - private final String name; +abstract sealed class LocalNodeIdImpl extends NodeIdImpl + permits InstrOrPhiIdImpl, AuxiliaryNodeIdImpl { private final String toString; - /** Create an instruction or phi id (of the given class, CFG, and name). */ - LocalNodeIdImpl(N node, String name) { - this(Classes.classOf(node), node.cfg().nextInstrOrPhiDisambiguator(), name, -1); + LocalNodeIdImpl(String toString) { + super(null); + this.toString = toString; } - /** Create an auxiliary node ID (of the given class, base ID, and auxiliary number). */ - LocalNodeIdImpl(N node, NodeId base, int auxiliary) { - super(Classes.classOf(node)); - if (!(base instanceof LocalNodeIdImpl b)) { - throw new AssertionError( - "auxiliary node ID can only be created for an instruction or phi ID, not a global node ID"); + /** + * Return the given {@link NodeId} casted to a {@link LocalNodeIdImpl}. + * + *

Any {@linkplain LocalNode local node} ID is guaranteed to be a {@link LocalNodeIdImpl}, so + * we have this dedicated method to reduce the amount of casts. + */ + @SuppressWarnings("unchecked") + static LocalNodeIdImpl cast(NodeId id) { + if (!(id instanceof LocalNodeIdImpl instrOrPhiId)) { + throw new ClassCastException( + "ID which is allegedly an instruction or phi ID (but maybe not due to generic erasure) isn't a `LocalNodeIdImpl`: " + + id); } - assert b.disambiguator == -1 - : "auxiliary node ID can only be created for an instruction or phi ID, not another auxiliary node ID"; - - name = base.name(); - toString = base + "#" + auxiliary; - this.disambiguator = b.disambiguator; - } - - private LocalNodeIdImpl( - @Nullable Class clazz, int disambiguator, String name, int auxiliary) { - super(clazz); - - this.name = name; - toString = - "%" - + disambiguator - + NodeAndBBIds.quoteNameIfNecessary(name) - + (auxiliary == -1 ? "" : "#" + auxiliary); - this.disambiguator = disambiguator; + return (LocalNodeIdImpl) instrOrPhiId; } /** @@ -170,16 +144,6 @@ void lateAssignClass(Class clazz) { this.clazz = (Class) clazz; } - /** Should only be used by {@link CFG}. */ - int disambiguator() { - return disambiguator; - } - - @Override - public String name() { - return name; - } - @Override public String toString() { return toString; @@ -190,26 +154,92 @@ public boolean isLocal() { return true; } + @ParseMethod(SkipWhitespace.NONE) + private static LocalNodeIdImpl parse(Parser p) { + var s = p.scanner(); + + s.assertAndSkip('%'); + var disambiguator = s.nextCharSatisfies(Character::isDigit) ? s.readUInt() : 0; + var name = NodeAndBBIds.readName(s); + var base = new InstrOrPhiIdImpl<>(disambiguator, name); + if (s.nextCharIs('#')) { + var auxiliary = s.readUInt(); + return new AuxiliaryNodeIdImpl<>(base, auxiliary); + } else { + return base; + } + } +} + +final class InstrOrPhiIdImpl extends LocalNodeIdImpl { + /** + * Return the given {@link NodeId} casted to a {@link InstrOrPhiIdImpl}. + * + *

Any {@linkplain InstrOrPhi instruction or phi} ID is guaranteed to be a {@link + * InstrOrPhiIdImpl}, so we have this dedicated method to reduce the amount of casts. + */ + @SuppressWarnings("unchecked") + static InstrOrPhiIdImpl cast(NodeId id) { + if (!(id instanceof InstrOrPhiIdImpl instrOrPhiId)) { + throw new ClassCastException( + "ID which is allegedly an instruction or phi ID (but maybe not due to generic erasure) isn't a `InstrOrPhiIdImpl`: " + + id); + } + return (InstrOrPhiIdImpl) instrOrPhiId; + } + + private final int disambiguator; + private final String name; + + InstrOrPhiIdImpl(int disambiguator, String name) { + super( + "%" + (disambiguator == 0 ? "" : disambiguator) + NodeAndBBIds.quoteNameIfNecessary(name)); + + this.disambiguator = disambiguator; + this.name = name; + } + + int disambiguator() { + return disambiguator; + } + + String name() { + return name; + } + @Override public boolean equals(Object o) { if (this == o) return true; - if (!(o instanceof LocalNodeIdImpl nodeId)) return false; - return disambiguator == nodeId.disambiguator; + if (!(o instanceof InstrOrPhiIdImpl that)) return false; + return disambiguator == that.disambiguator && name.equals(that.name); } @Override public int hashCode() { - return Objects.hash(disambiguator); + return Objects.hash(disambiguator == 0 ? name : disambiguator); } +} - @ParseMethod(SkipWhitespace.NONE) - private static LocalNodeIdImpl parse(Parser p) { - var s = p.scanner(); +final class AuxiliaryNodeIdImpl extends LocalNodeIdImpl { + private final InstrOrPhiIdImpl owner; + private final int index; - s.assertAndSkip('%'); - var disambiguator = s.readUInt(); - var name = NodeAndBBIds.readName(s); - var auxiliary = s.trySkip('#') ? s.readUInt() : -1; - return new LocalNodeIdImpl<>(null, disambiguator, name, auxiliary); + AuxiliaryNodeIdImpl(InstrOrPhiIdImpl owner, int index) { + super(owner + "#" + index); + + this.owner = owner; + this.index = index; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof AuxiliaryNodeIdImpl that)) return false; + return owner.equals(that.owner) && index == that.index; + } + + @Override + public int hashCode() { + return Objects.hash(owner, index); } } diff --git a/src/main/java/org/prlprg/ir/cfg/NodeOrBBIdDisambiguatorMap.java b/src/main/java/org/prlprg/ir/cfg/NodeOrBBIdDisambiguatorMap.java new file mode 100644 index 000000000..08f1e268d --- /dev/null +++ b/src/main/java/org/prlprg/ir/cfg/NodeOrBBIdDisambiguatorMap.java @@ -0,0 +1,64 @@ +package org.prlprg.ir.cfg; + +import java.util.HashMap; +import java.util.Map; +import java.util.NavigableSet; +import java.util.TreeSet; + +/** + * Map that stores names and disambiguators for {@link NodeId}s ({@link InstrOrPhiIdImpl}s) and + * {@link BBId}s ({@link BBIdImpl}s), so that a low disambiguator can be returned that actually + * disambiguates said name. + * + *

Specifically, it stores every disambiguator for each name, and generating a unique + * disambiguator for a name returns one that is the successor to the largest stored one. + */ +class NodeOrBBIdDisambiguatorMap { + private final Map> disambiguators = new HashMap<>(); + + NodeOrBBIdDisambiguatorMap() { + // Ensure that the an empty string always has a (positive) disambiguator. + add("", 0); + } + + /** + * Get a unique disambiguator for the given name. + * + *

This doesn't insert the disambiguator, so a subsequent call will return the same + * value. The disambiguator is only stored when {@link #add(String, int)} is called. + */ + int get(String name) { + return disambiguators.containsKey(name) ? disambiguators.get(name).last() + 1 : 0; + } + + /** + * Record that the given disambiguator is used for the given name. + * + *

Specifically, this makes it so {@link #get(String)} is guaranteed not to return the given + * disambiguator, until {@link #remove(String, int)} is called. + * + * @throws IllegalArgumentException If the given disambiguator is already used for the given name. + */ + void add(String name, int disambiguator) { + if (!disambiguators.computeIfAbsent(name, _ -> new TreeSet<>()).add(disambiguator)) { + throw new IllegalArgumentException( + "Disambiguator " + disambiguator + " is already used for name " + name); + } + } + + /** + * Remove the given disambiguator from the given name. + * + *

Specifically, this makes it so {@link #get(String)} may return the given disambiguator. It + * may not though; specifically, if a higher disambiguator still exists, it will still return a + * disambiguator greater than that one. + * + * @throws IllegalArgumentException If the given disambiguator is not used for the given name. + */ + void remove(String name, int disambiguator) { + if (!disambiguators.computeIfAbsent(name, _ -> new TreeSet<>()).remove(disambiguator)) { + throw new IllegalArgumentException( + "Disambiguator " + disambiguator + " is not used for name " + name); + } + } +} diff --git a/src/main/java/org/prlprg/ir/cfg/Phi.java b/src/main/java/org/prlprg/ir/cfg/Phi.java index 5b55b7b02..464fb3224 100644 --- a/src/main/java/org/prlprg/ir/cfg/Phi.java +++ b/src/main/java/org/prlprg/ir/cfg/Phi.java @@ -8,13 +8,7 @@ import java.util.Comparator; import java.util.SequencedCollection; import java.util.SequencedSet; -import javax.annotation.Nullable; import org.jetbrains.annotations.UnmodifiableView; -import org.prlprg.ir.cfg.CFGEdit.RenameInstrOrPhi; -import org.prlprg.parseprint.ParseMethod; -import org.prlprg.parseprint.Parser; -import org.prlprg.parseprint.PrintMethod; -import org.prlprg.parseprint.Printer; import org.prlprg.util.SequencedCollections; import org.prlprg.util.SmallBinarySet; @@ -232,7 +226,11 @@ public String toString() { } } -abstract class PhiImpl implements Phi { +abstract non-sealed class PhiImpl extends InstrOrPhiImpl implements Phi { + private final Class nodeClass; + private final SmallBinarySet> inputs; + + // region construct /** * Constructor arguments that can be stored in a collection (since there are multiple; * alternatively could use {@link org.prlprg.util.Pair} but this is clearer). @@ -241,11 +239,11 @@ abstract class PhiImpl implements Phi { * functionality is only used in {@link CFGEdit}s that are replayed to maintain determinism. */ record Args( - @Nullable NodeId> id, + NodeId> id, Class nodeClass, Collection> inputs) { - Args(Phi.Args args) { - this(null, args.nodeClass(), args.inputs()); + Args(NodeId> id, Phi.Args args) { + this(id, args.nodeClass(), args.inputs()); } Args(CFG cfg, Serial serial) { @@ -260,11 +258,6 @@ record Args( } } - private final Class nodeClass; - private final CFG cfg; - private LocalNodeIdImpl id; - private final SmallBinarySet> inputs; - /** * Create a new phi-node for nodes of the given class. * @@ -277,17 +270,17 @@ record Args( */ @SuppressWarnings("unchecked") static Phi forClass( - @Nullable NodeId> presetId, Class nodeSubclass, CFG cfg, + NodeId> id, Collection> inputs) { Phi phi; if (RValue.class.isAssignableFrom(nodeSubclass)) { - phi = new RValuePhiImpl(cfg, presetId, inputs); + phi = new RValuePhiImpl(cfg, id, inputs); } else if (DeoptReason.class.isAssignableFrom(nodeSubclass)) { - phi = new DeoptReasonPhiImpl(cfg, presetId, inputs); + phi = new DeoptReasonPhiImpl(cfg, id, inputs); } else if (FrameState.class.isAssignableFrom(nodeSubclass)) { - phi = new FrameStatePhiImpl(cfg, presetId, inputs); + phi = new FrameStatePhiImpl(cfg, id, inputs); } else { throw new UnsupportedOperationException( "No phi type implemented for the given class: " + nodeSubclass); @@ -295,12 +288,23 @@ static Phi forClass( return (Phi) phi; } + /** + * Return the given phi casted. + * + *

Any {@link Phi} is guaranteed to be an {@link PhiImpl}, so this method is provided to reduce + * the number of casts in the code text. + */ + static PhiImpl cast(Phi phi) { + return (PhiImpl) phi; + } + @SuppressWarnings("unchecked") protected PhiImpl( Class nodeClass, CFG cfg, - @Nullable NodeId presetId, + NodeId> id, Collection> inputs) { + super(cfg, id); this.inputs = new SmallBinarySet<>( inputs.size(), Comparator.comparing(i -> i.incomingBB().id().toString())); @@ -310,21 +314,6 @@ protected PhiImpl( this.inputs.add((Input) input); } this.nodeClass = nodeClass; - this.cfg = cfg; - - if (presetId == null) { - id = new LocalNodeIdImpl<>(this, ""); - } else { - if (!(presetId instanceof LocalNodeIdImpl i)) { - throw new IllegalArgumentException( - "Node a a phi ID: " - + presetId - + "\nTo get this, you may have improperly casted a NodeId's generic argument."); - } - i.lateAssignClass(getClass()); - cfg.updateNextInstrOrPhiDisambiguator((NodeId) i); - id = i; - } for (var inputNode : inputNodes()) { if (!nodeClass.isInstance(inputNode)) { @@ -337,18 +326,18 @@ protected PhiImpl( + nodeClass); } } - for (var input : inputs) { - verifyInputIfEagerConfig(input); - } assert nodeClass.isAssignableFrom(InvalidNode.class) : "InvalidNode should be an instance of any phi class"; } + // endregion construct + @Override public Class nodeClass() { return nodeClass; } + // region inputs @Override public SequencedSet> inputs() { return inputs; @@ -403,7 +392,7 @@ public N setInput(BB incomingBB, N node) { if (index == -1) { throw new IllegalArgumentException( "incoming BB not a predecessor of " - + id + + id() + "'s BB: " + incomingBB.id() + " (node = " @@ -413,7 +402,7 @@ public N setInput(BB incomingBB, N node) { if (!nodeClass.isInstance(node)) { throw new IllegalArgumentException( "Tried to add an input of incompatible type to " - + id + + id() + ": " + node.id() + " (" @@ -421,17 +410,35 @@ public N setInput(BB incomingBB, N node) { + ") is not an instance of the phi's node class " + nodeClass.getSimpleName()); } - verifyInputIfEagerConfig(new Input<>(incomingBB, node)); + if (EAGERLY_VERIFY_PHI_INPUTS) { + eagerlyVerifyInput(new Input<>(incomingBB, node)); + } var oldNode = inputs.get(index).node(); inputs.equalReplace(index, Input.of(incomingBB, node)); - cfg.record( - new CFGEdit.SetPhiInput<>(this, incomingBB, node), - new CFGEdit.SetPhiInput<>(this, incomingBB, oldNode)); + cfg() + .record( + new CFGEdit.SetPhiInput<>(this, incomingBB, node), + new CFGEdit.SetPhiInput<>(this, incomingBB, oldNode)); return oldNode; } + void eagerlyVerifyInputs() { + for (var input : inputs) { + eagerlyVerifyInput(input); + } + } + + private void eagerlyVerifyInput(Phi.Input input) { + var issue = cfg().verifyPhiInput(this, input, true); + if (issue != null) { + throw new CFGVerifyException(cfg(), issue); + } + } + + // endregion inputs + @SuppressWarnings("unchecked") @Override public void replaceInArgs(Node old, Node replacement) { @@ -448,60 +455,16 @@ public void replaceInArgs(Node old, Node replacement) { var input = inputs.get(i); if (input.node().equals(old)) { inputs.equalReplace(i, Input.of(input.incomingBB(), (N) replacement)); - cfg.record( - new CFGEdit.SetPhiInput<>(this, input.incomingBB(), (N) replacement), - new CFGEdit.SetPhiInput<>(this, input.incomingBB(), (N) old)); - } - } - } - - private void verifyInputIfEagerConfig(Phi.Input input) { - if (EAGERLY_VERIFY_PHI_INPUTS) { - var issue = cfg().verifyPhiInput(this, input, true); - if (issue != null) { - throw new CFGVerifyException(cfg(), issue); + cfg() + .record( + new CFGEdit.SetPhiInput<>(this, input.incomingBB(), (N) replacement), + new CFGEdit.SetPhiInput<>(this, input.incomingBB(), (N) old)); } } } - @Override - public final void rename(String newName) { - var oldId = id(); - - cfg().untrack(this); - id = new LocalNodeIdImpl<>(this, newName); - cfg().track(this); - - cfg().record(new RenameInstrOrPhi(oldId, newName), new RenameInstrOrPhi(id(), oldId.name())); - } - - @Override - public CFG cfg() { - return cfg; - } - @Override public NodeId> id() { return uncheckedCastId(); } - - @SuppressWarnings("unchecked") - protected > NodeId uncheckedCastId() { - return (NodeId) id; - } - - @Override - public String toString() { - return Printer.toString(this); - } - - @ParseMethod - private Phi parse(Parser p) { - throw new UnsupportedOperationException("can't parse a Phi outside of a BB"); - } - - @PrintMethod - private void print(Printer p) { - p.withContext(new CFGParseOrPrintContext(p.context(), cfg()).new BBContext(null)).print(this); - } } diff --git a/src/main/java/org/prlprg/ir/cfg/RContext.java b/src/main/java/org/prlprg/ir/cfg/RContext.java index bce22293e..73933e3b2 100644 --- a/src/main/java/org/prlprg/ir/cfg/RContext.java +++ b/src/main/java/org/prlprg/ir/cfg/RContext.java @@ -19,8 +19,8 @@ public interface RContext extends Stmt { } final class RContextImpl extends SelfReturningStmtImpl implements RContext { - RContextImpl(CFG cfg, TokenToCreateNewInstr token, PushContext data) { - super(PushContext.class, cfg, token, data); + RContextImpl(CFG cfg, NodeId id, PushContext data) { + super(PushContext.class, cfg, id, data); } @Override diff --git a/src/main/java/org/prlprg/ir/cfg/RValuePhi.java b/src/main/java/org/prlprg/ir/cfg/RValuePhi.java index 516cf2617..880050ec9 100644 --- a/src/main/java/org/prlprg/ir/cfg/RValuePhi.java +++ b/src/main/java/org/prlprg/ir/cfg/RValuePhi.java @@ -2,7 +2,6 @@ import java.util.Collection; import java.util.HashSet; -import javax.annotation.Nullable; import org.prlprg.ir.type.RType; import org.prlprg.ir.type.RTypes; @@ -17,8 +16,8 @@ public interface RValuePhi extends Phi, RValue { } final class RValuePhiImpl extends PhiImpl implements RValuePhi { - RValuePhiImpl(CFG cfg, @Nullable NodeId presetId, Collection> inputs) { - super(RValue.class, cfg, presetId, inputs); + RValuePhiImpl(CFG cfg, NodeId> id, Collection> inputs) { + super(RValue.class, cfg, id, inputs); } @Override diff --git a/src/main/java/org/prlprg/ir/cfg/RValueStmt.java b/src/main/java/org/prlprg/ir/cfg/RValueStmt.java index a7edb9a0f..3340794e3 100644 --- a/src/main/java/org/prlprg/ir/cfg/RValueStmt.java +++ b/src/main/java/org/prlprg/ir/cfg/RValueStmt.java @@ -30,8 +30,8 @@ abstract class AbstractRValueStmtImpl extends SelfRe private @Nullable EnvAux envAux; - AbstractRValueStmtImpl(Class clazz, CFG cfg, TokenToCreateNewInstr token, D data) { - super(clazz, cfg, token, data); + AbstractRValueStmtImpl(Class clazz, CFG cfg, NodeId id, D data) { + super(clazz, cfg, id, data); } @Override @@ -81,7 +81,7 @@ public NodeId id() { } final class RValueStmtImpl extends AbstractRValueStmtImpl { - RValueStmtImpl(CFG cfg, TokenToCreateNewInstr token, StmtData.RValue_ data) { - super(StmtData.RValue_.class, cfg, token, data); + RValueStmtImpl(CFG cfg, NodeId id, StmtData.RValue_ data) { + super(StmtData.RValue_.class, cfg, id, data); } } diff --git a/src/main/java/org/prlprg/ir/cfg/Stmt.java b/src/main/java/org/prlprg/ir/cfg/Stmt.java index bd592587e..095bc51c6 100644 --- a/src/main/java/org/prlprg/ir/cfg/Stmt.java +++ b/src/main/java/org/prlprg/ir/cfg/Stmt.java @@ -41,15 +41,7 @@ abstract non-sealed class StmtImpl> extends InstrImpl i *

Unlike {@link Stmt.Args}, this exposes that statements can be created with existing IDs. * Such functionality is only used in {@link CFGEdit}s that are replayed to maintain determinism. */ - record Args(TokenToCreateNewInstr token, StmtData data) { - Args(Stmt.Args args) { - this(new CreateInstrWithNewId(args.name()), args.data()); - } - - private Args(NodeId id, StmtData data) { - this(new CreateInstrWithExistingId(id), data); - } - + record Args(NodeId id, StmtData data) { Args(CFG cfg, Serial serial) { this(serial.id(), serial.data().decode(cfg)); } @@ -59,8 +51,8 @@ private Args(NodeId id, StmtData data) { } } - StmtImpl(Class dataClass, CFG cfg, TokenToCreateNewInstr token, D data) { - super(dataClass, cfg, token, data); + StmtImpl(Class dataClass, CFG cfg, NodeId id, D data) { + super(dataClass, cfg, id, data); } @Override @@ -71,8 +63,8 @@ public NodeId id() { /** {@link Stmt} (IR instruction) which doesn't produce anything. */ final class VoidStmtImpl extends StmtImpl { - VoidStmtImpl(CFG cfg, TokenToCreateNewInstr token, StmtData.Void data) { - super(StmtData.Void.class, cfg, token, data); + VoidStmtImpl(CFG cfg, NodeId id, StmtData.Void data) { + super(StmtData.Void.class, cfg, id, data); } @Override @@ -85,8 +77,8 @@ public ImmutableList returns() { abstract class SelfReturningStmtImpl> extends StmtImpl { private final ImmutableList returns = ImmutableList.of(this); - SelfReturningStmtImpl(Class dataClass, CFG cfg, TokenToCreateNewInstr token, D data) { - super(dataClass, cfg, token, data); + SelfReturningStmtImpl(Class dataClass, CFG cfg, NodeId id, D data) { + super(dataClass, cfg, id, data); } @Override diff --git a/src/main/java/org/prlprg/ir/cfg/StmtData.java b/src/main/java/org/prlprg/ir/cfg/StmtData.java index 758bb3009..294b36b8c 100644 --- a/src/main/java/org/prlprg/ir/cfg/StmtData.java +++ b/src/main/java/org/prlprg/ir/cfg/StmtData.java @@ -30,8 +30,8 @@ public sealed interface StmtData extends InstrData { sealed interface Void extends StmtData { @Override - default Stmt make(CFG cfg, TokenToCreateNewInstr token) { - return new VoidStmtImpl(cfg, token, this); + default Stmt make(CFG cfg, NodeId id) { + return new VoidStmtImpl(cfg, id, this); } } @@ -53,8 +53,8 @@ sealed interface RValue_ extends StmtData { } @Override - default RValueStmt make(CFG cfg, TokenToCreateNewInstr token) { - return new RValueStmtImpl(cfg, token, this); + default RValueStmt make(CFG cfg, NodeId id) { + return new RValueStmtImpl(cfg, id, this); } } @@ -83,8 +83,8 @@ sealed interface Call_ extends RValue_, InstrData.HasAst { RValue env(); @Override - default org.prlprg.ir.cfg.Call make(CFG cfg, TokenToCreateNewInstr token) { - return new CallImpl(cfg, token, this); + default org.prlprg.ir.cfg.Call make(CFG cfg, NodeId id) { + return new CallImpl(cfg, id, this); } } @@ -149,8 +149,8 @@ public FrameState( } @Override - public FrameStateStmt make(CFG cfg, TokenToCreateNewInstr token) { - return new FrameStateStmtImpl(cfg, token, this); + public FrameStateStmt make(CFG cfg, NodeId id) { + return new FrameStateStmtImpl(cfg, id, this); } } @@ -1029,8 +1029,8 @@ public PushContext( } @Override - public RContext make(CFG cfg, TokenToCreateNewInstr token) { - return new RContextImpl(cfg, token, this); + public RContext make(CFG cfg, NodeId id) { + return new RContextImpl(cfg, id, this); } } diff --git a/src/main/java/org/prlprg/ir/cfg/TokenToCreateNewInstr.java b/src/main/java/org/prlprg/ir/cfg/TokenToCreateNewInstr.java deleted file mode 100644 index 1883ae176..000000000 --- a/src/main/java/org/prlprg/ir/cfg/TokenToCreateNewInstr.java +++ /dev/null @@ -1,17 +0,0 @@ -package org.prlprg.ir.cfg; - -/** - * Token to create an {@linkplain Instr instruction}, with either an existing* or new {@linkplain - * NodeId id}. - * - *

This class's only instances are package-private and not exposed, so that {@link - * InstrData#make(CFG, TokenToCreateNewInstr)} can only be called within the package. - * - *

* The only way to create a new instruction with an existing ID is by replaying an {@linkplain - * CFGEdit edit}. - */ -public sealed interface TokenToCreateNewInstr {} - -record CreateInstrWithExistingId(NodeId id) implements TokenToCreateNewInstr {} - -record CreateInstrWithNewId(String name) implements TokenToCreateNewInstr {}