Skip to content

Commit

Permalink
refactor BB and node IDs *again*
Browse files Browse the repository at this point in the history 
Now they won't have a disambiguator (number prefix) if it's unnecessary, and will get lower ones.

TODO add a step in cleanup which re-assigns instructions IDs with the lowest disambiguators possible.

Otherwise this is hopefully the last BB and node ID refactor.

Also, all `ClosureCompilerTests` before `switch` up to builtins pass now, and all other tests pass.
  • Loading branch information
@Jakobeha
Jakobeha committed Jun 22, 2024
1 parent 0cecacb commit 237c3f4
Show file tree
Hide file tree
Showing 29 changed files with 688 additions and 514 deletions.
2 changes: 1 addition & 1 deletion src/main/java/org/prlprg/ir/cfg/Assumption.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ public InstrOrPhi origin() {

@Override
public NodeId<? extends Assumption> id() {
return new LocalNodeIdImpl<>(this, checkpoint.id(), index);
return new AuxiliaryNodeIdImpl<>(InstrOrPhiIdImpl.cast(checkpoint.id()), index);
}
}
135 changes: 80 additions & 55 deletions src/main/java/org/prlprg/ir/cfg/BB.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -356,15 +358,15 @@ 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)
: "BB has successor whose predecessors set doesn't contain it";
succ.predecessors.remove(this);
succ.predecessors.add(newBB);
for (var phi : succ.phis) {
((PhiImpl<?>) phi).unsafeReplaceIncomingBB(this, newBB);
PhiImpl.cast(phi).unsafeReplaceIncomingBB(this, newBB);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand All @@ -432,27 +434,20 @@ public void mergeWithSuccessor(BB succ) {
// region add nodes
@Override
public <N extends Node> Phi<N> addPhi(Class<? extends N> nodeClass) {
var phi =
PhiImpl.<N>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().<Phi<? extends N>>uniqueInstrOrPhiId(), nodeClass);
}

@Override
public <N extends Node> Phi<N> addPhi(
Class<? extends N> nodeClass, Collection<? extends Phi.Input<? extends N>> inputs) {
return addPhiWithId(null, nodeClass, inputs);
return addPhiWithId(cfg().<Phi<? extends N>>uniqueInstrOrPhiId(), nodeClass, inputs);
}

<N extends Node> Phi<N> addPhiWithId(
@Nullable NodeId<? extends Phi<? extends N>> id, Class<? extends N> nodeClass) {
NodeId<? extends Phi<? extends N>> id, Class<? extends N> 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);

Expand All @@ -461,15 +456,22 @@ <N extends Node> Phi<N> addPhiWithId(
}

<N extends Node> Phi<N> addPhiWithId(
@Nullable NodeId<? extends Phi<? extends N>> id,
NodeId<? extends Phi<? extends N>> id,
Class<? extends N> nodeClass,
Collection<? extends Phi.Input<? extends N>> 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;
}

Expand All @@ -479,7 +481,9 @@ public ImmutableList<? extends Phi<?>> addPhis(
var inputs = predecessors.stream().map(Phi.Input::unset).toList();
var phis =
nodeClasses.stream()
.map(nodeClass -> PhiImpl.<Node>forClass(null, nodeClass, cfg(), inputs))
.map(
nodeClass ->
PhiImpl.forClass(nodeClass, cfg(), cfg().<Phi<?>>uniqueInstrOrPhiId(), inputs))
.collect(ImmutableList.toImmutableList());
this.phis.addAll(phis);
cfg().trackAll(phis);
Expand All @@ -490,13 +494,18 @@ public ImmutableList<? extends Phi<?>> addPhis(

@Override
public ImmutableList<? extends Phi<?>> addPhis1(Collection<Phi.Args> phiArgs) {
return addPhisWithIds(phiArgs.stream().map(PhiImpl.Args::new).toList());
return addPhisWithIds(
phiArgs.stream()
.map(a -> new PhiImpl.Args(cfg().<Phi<?>>uniqueInstrOrPhiId(), a))
.toList());
}

ImmutableList<? extends Phi<?>> addPhisWithIds(Collection<? extends PhiImpl.Args> 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);
Expand All @@ -505,6 +514,15 @@ ImmutableList<? extends Phi<?>> addPhisWithIds(Collection<? extends PhiImpl.Args
cfg().trackAll(phis);

cfg().record(new CFGEdit.InsertPhis(this, phis), new CFGEdit.RemovePhis(this, phis));

// 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) {
for (var phi : phis) {
PhiImpl.cast(phi).eagerlyVerifyInputs();
}
}

return phis;
}

Expand All @@ -524,15 +542,14 @@ private void verifyPhiInputBBs(Phi<?> phi) {

@Override
public <I extends Stmt> I insertAt(int index, String name, StmtData<I> args) {
return insertAtWithToken(index, new CreateInstrWithNewId(name), args);
return insertAtWithId(index, cfg().<I>uniqueInstrOrPhiId(name), args);
}

<I extends Stmt> I insertAtWithToken(
int index, TokenToCreateNewInstr token, StmtData<? extends I> args) {
<I extends Stmt> I insertAtWithId(int index, NodeId<? extends I> id, StmtData<? extends I> 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);

Expand All @@ -542,17 +559,20 @@ <I extends Stmt> I insertAtWithToken(

@Override
public ImmutableList<? extends Stmt> insertAllAt(int index, List<Stmt.Args> namesAndArgs) {
return insertAllAtWithTokens(index, namesAndArgs.stream().map(StmtImpl.Args::new).toList());
return insertAllAtWithIds(
index,
namesAndArgs.stream()
.map(a -> new StmtImpl.Args(cfg().<Stmt>uniqueInstrOrPhiId(a.name()), a.data()))
.toList());
}

ImmutableList<? extends Stmt> insertAllAtWithTokens(
int index, List<StmtImpl.Args> tokensAndArgs) {
ImmutableList<? extends Stmt> insertAllAtWithIds(int index, List<StmtImpl.Args> 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);
Expand All @@ -566,14 +586,19 @@ ImmutableList<? extends Stmt> insertAllAtWithTokens(

@Override
public ImmutableList<? extends Stmt> insertAllAt(Map<Integer, Stmt.Args> 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().<Stmt>uniqueInstrOrPhiId(e.getValue().name()),
e.getValue().data()))));
}

ImmutableList<? extends Stmt> insertAllAtWithTokens(
Map<Integer, StmtImpl.Args> indicesTokensAndArgs) {
for (var index : indicesTokensAndArgs.keySet()) {
ImmutableList<? extends Stmt> insertAllAtWithIds(Map<Integer, StmtImpl.Args> indicesIdsAndArgs) {
for (var index : indicesIdsAndArgs.keySet()) {
if (index < 0 || index > stmts.size()) {
throw new IndexOutOfBoundsException("Index out of range: " + index);
}
Expand All @@ -583,14 +608,14 @@ ImmutableList<? extends Stmt> 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.<Stmt>builderWithExpectedSize(indicesTokensAndArgs.size());
var newStmtsBuilder = ImmutableList.<Stmt>builderWithExpectedSize(indicesIdsAndArgs.size());
var indicesStmts =
ImmutableMap.<Integer, Stmt>builderWithExpectedSize(indicesTokensAndArgs.size());
ImmutableMap.<Integer, Stmt>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);
Expand All @@ -611,14 +636,14 @@ ImmutableList<? extends Stmt> insertAllAtWithTokens(

@Override
public <I extends Jump> I addJump(String name, JumpData<I> args) {
return addJumpWithToken(new CreateInstrWithNewId(name), args);
return addJumpWithId(cfg().<I>uniqueInstrOrPhiId(name), args);
}

<I extends Jump> I addJumpWithToken(TokenToCreateNewInstr token, JumpData<? extends I> args) {
<I extends Jump> I addJumpWithId(NodeId<? extends I> id, JumpData<? extends I> 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);

Expand All @@ -636,19 +661,19 @@ public <I extends Stmt> 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().<I>uniqueInstrOrPhiId(newName), newArgs);
}

<I extends Stmt> I replaceWithToken(
int index, TokenToCreateNewInstr newIdToken, StmtData<? extends I> newArgs) {
<I extends Stmt> I replaceWithId(
int index, NodeId<? extends I> newId, StmtData<? extends I> 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);
Expand All @@ -668,19 +693,19 @@ public <I extends Jump> I replaceJump(@Nullable String newName, JumpData<I> 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().<I>uniqueInstrOrPhiId(newName), newArgs);
}

public <I extends Jump> I replaceJumpWithToken(
TokenToCreateNewInstr newIdToken, JumpData<? extends I> newArgs) {
public <I extends Jump> I replaceJumpWithId(
NodeId<? extends I> newId, JumpData<? extends I> 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);
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down
Loading

0 comments on commit 237c3f4

Please sign in to comment.