Skip to content

Commit 95b2cf0

Browse files
authored
Inlining: Handle local dependencies when splitting (#8064)
In pattern B there, we handle stuff like if (..) { .. } return x; We split out the if body if it is large-enough, which allows inlining the if condition + the return (efficient if the condition rarely happens). This did not handle local effects: imagine that the if body contains x = 42, then after splitting it out to another function, that value is not picked up in the return of x. Fix that by checking local dependencies. More detailed example: function foo(x) { if (..) { work(); x = 42; } return x; } function caller() { foo(1); } => the incorrect optimization before => function caller() { // inlined call, but split: just the condition + return. var x = 1; // inlined value sent in call if (..) { outlinedCode(); } x = x; } function outlinedCode() { // The setting of x to 42 is done here, and not picked up // in the caller. var x; work(); x = 42; } After this PR, we do not do such split inlining.
1 parent df3d896 commit 95b2cf0

File tree

2 files changed

+513
-1
lines changed

2 files changed

+513
-1
lines changed

src/passes/Inlining.cpp

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -978,7 +978,19 @@ struct FunctionSplitter {
978978
if (finalItem && getItem(body, numIfs + 1)) {
979979
return InliningMode::Uninlineable;
980980
}
981-
// This has the general shape we seek. Check each if.
981+
// This has the general shape we seek. Check each if: it must be in the
982+
// form mentioned above (simple condition, no returns in body). We must also
983+
// have no sets of locals that the final item notices, as then we could
984+
// have this:
985+
//
986+
// if (A) {
987+
// x = 10;
988+
// }
989+
// return x;
990+
//
991+
// We cannot split out the if in such a case because of the local
992+
// dependency.
993+
std::unordered_set<Index> writtenLocals;
982994
for (Index i = 0; i < numIfs; i++) {
983995
auto* iff = getIf(body, i);
984996
// The if must have a simple condition and no else arm.
@@ -995,7 +1007,21 @@ struct FunctionSplitter {
9951007
// unreachable, and we ruled out none before.
9961008
assert(iff->ifTrue->type == Type::unreachable);
9971009
}
1010+
if (finalItem) {
1011+
for (auto* set : FindAll<LocalSet>(iff).list) {
1012+
writtenLocals.insert(set->index);
1013+
}
1014+
}
9981015
}
1016+
// Finish the locals check mentioned above.
1017+
if (finalItem) {
1018+
for (auto* get : FindAll<LocalGet>(finalItem).list) {
1019+
if (writtenLocals.count(get->index)) {
1020+
return InliningMode::Uninlineable;
1021+
}
1022+
}
1023+
}
1024+
9991025
// Success, this matches the pattern.
10001026

10011027
// If the outlined function will be worth inlining normally, skip the

0 commit comments

Comments
 (0)