Skip to content

Commit

Permalink
Fix incorrect ppmm inlining (#451)
Browse files Browse the repository at this point in the history
* Add bad test for ppmm inlining

* Update test

* Fix ppmm bug

* More bad case :(

* Fix array assign ppmm bug
  • Loading branch information
Kroppeb committed Feb 22, 2025
1 parent 07ca6db commit 0be304f
Show file tree
Hide file tree
Showing 5 changed files with 446 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -683,20 +683,30 @@ private static boolean inlinePPIAndMMI(Exprent expr, Exprent next) {
// Try to find the first valid usage of a variable for PPMM inlining.
// Returns Pair{parent exprent, variable exprent to replace}
private static Pair<Exprent, VarExprent> findFirstValidUsage(VarExprent match, Exprent next) {
Deque<Exprent> stack = new LinkedList<>();
List<Exprent> stack = new ArrayList<>();
List<Exprent> parent = new ArrayList<>();
stack.add(next);
parent.add(null);

while (!stack.isEmpty()) {
Exprent expr = stack.removeLast();
Exprent expr = stack.remove(stack.size() - 1);
Exprent parentExpr = parent.remove(parent.size() - 1);

List<Exprent> exprents = expr.getAllExprents();

if (parentExpr != null &&
expr instanceof VarExprent ve &&
ve.getIndex() == match.getIndex() &&
ve.getVersion() == match.getVersion()) {
return Pair.of(parentExpr, ve);
}

if (expr instanceof FunctionExprent) {
FunctionExprent func = (FunctionExprent) expr;

// Don't inline ppmm into more ppmm
if (isPPMM(func)) {
continue;
return null;
}

// Don't consider || or &&
Expand All @@ -718,30 +728,18 @@ private static Pair<Exprent, VarExprent> findFirstValidUsage(VarExprent match, E
// Reverse iteration to ensure DFS
for (int i = exprents.size() - 1; i >= 0; i--) {
Exprent ex = exprents.get(i);
boolean add = true;

// Skip LHS of assignment as it is invalid
if (expr instanceof AssignmentExprent) {
add = ex != ((AssignmentExprent) expr).getLeft();
}

// Check var if we find
if (add && ex instanceof VarExprent) {
VarExprent ve = (VarExprent) ex;

if (ve.getIndex() == match.getIndex() && ve.getVersion() == match.getVersion()) {
return Pair.of(expr, ve);
}
}

// Ignore ++/-- exprents as they aren't valid usages to replace
if (ex instanceof FunctionExprent) {
add = !isPPMM((FunctionExprent) ex);
// Avoid making something like `++a = 5`. It shouldn't happen but better be safe than sorry.
if (expr instanceof AssignmentExprent asExpr &&
ex == asExpr.getLeft() &&
ex instanceof VarExprent innerEx &&
innerEx.getIndex() == match.getIndex()
) {
continue;
}

if (add) {
stack.add(ex);
}
stack.add(ex);
parent.add(expr);
}
}

Expand Down
52 changes: 49 additions & 3 deletions testData/results/pkg/TestArrayPPMM.dec
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,20 @@ public class TestArrayPPMM {
this.accept(array[i++], array[++i]);// 39
}// 40

private void accept(int i, int j) {
public void test9(int[] array, int i) {
array[++i] = i;// 43
}// 44

public void test10(int i) {
this.getArray()[++i] = i;// 47
}// 48

private void accept(int i, int j) {
}// 52

private int[] getArray() {
return new int[10];// 55
}
}

class 'pkg/TestArrayPPMM' {
Expand Down Expand Up @@ -190,8 +202,37 @@ class 'pkg/TestArrayPPMM' {
10 37
}

method 'accept (II)V' {
method 'test9 ([II)V' {
0 40
1 40
2 40
3 40
5 40
6 40
7 41
}

method 'test10 (I)V' {
0 44
1 44
2 44
3 44
4 44
5 44
6 44
8 44
9 44
a 45
}

method 'accept (II)V' {
0 48
}

method 'getArray ()[I' {
0 51
1 51
4 51
}
}

Expand All @@ -214,4 +255,9 @@ Lines mapping:
36 <-> 34
39 <-> 37
40 <-> 38
44 <-> 41
43 <-> 41
44 <-> 42
47 <-> 45
48 <-> 46
52 <-> 49
55 <-> 52
Loading

0 comments on commit 0be304f

Please sign in to comment.