[llvm] d5244fb - [WebAssembly] Use SSAUpdaterBulk in LowerEmscriptenSjLj
Heejin Ahn via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 24 18:24:08 PDT 2021
Author: Heejin Ahn
Date: 2021-08-24T18:23:51-07:00
New Revision: d5244fb16070068fc1dfae5804ab8ff86d25deb9
URL: https://github.com/llvm/llvm-project/commit/d5244fb16070068fc1dfae5804ab8ff86d25deb9
DIFF: https://github.com/llvm/llvm-project/commit/d5244fb16070068fc1dfae5804ab8ff86d25deb9.diff
LOG: [WebAssembly] Use SSAUpdaterBulk in LowerEmscriptenSjLj
We update SSA in two steps in Emscripten SjLj:
1. Rewrite uses of `setjmpTable` and `setjmpTableSize` variables and
place `phi`s where necessary, which are updated where we call
`saveSetjmp`.
2. Do a whole function level SSA update for all variables, because we
split BBs where `setjmp` is called and there are possibly variable
uses that are not dominated by a def.
(See https://github.com/llvm/llvm-project/blob/955b91c19c00ed4c917559a5d66d14c669dde2e3/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L1314-L1324)
We have been using `SSAUpdater` to do this, but `SSAUpdaterBulk` class
was added after this pass was first created, and for the step 2 it looks
like a better alternative with a possible performance benefit. Not sure
the author is aware of it, but `SSAUpdaterBulk` seems to have a
limitation: it cannot handle a use within the same BB as a def but
before it. For example:
```
... = %a + 1
%a = foo();
```
or
```
%a = %a + 1
```
The uses `%a` in RHS should be rewritten with another SSA variable of
`%a`, most likely one generated from a `phi`. But `SSAUpdaterBulk`
thinks all uses of `%a` are below the def of `%a` within the same BB.
(`SSAUpdater` has two different functions of rewriting because of this:
`RewriteUse` and `RewriteUseAfterInsertions`.) This doesn't affect our
usage in the step 2 because that deals with possibly non-dominated uses
by defs after block splitting. But it does in the step 1, which still
uses `SSAUpdater`.
But this CL also simplifies the step 1 by using `make_early_inc_range`,
removing the need to advance the iterator before rewriting a use.
This is NFC; the test changes are just the order of PHI nodes.
Reviewed By: dschuff
Differential Revision: https://reviews.llvm.org/D108583
Added:
Modified:
llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
index 01aae067541b..3488ef42cfb9 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
@@ -200,6 +200,7 @@
#include "llvm/Support/CommandLine.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/SSAUpdater.h"
+#include "llvm/Transforms/Utils/SSAUpdaterBulk.h"
using namespace llvm;
@@ -641,25 +642,23 @@ void WebAssemblyLowerEmscriptenEHSjLj::wrapTestSetjmp(
void WebAssemblyLowerEmscriptenEHSjLj::rebuildSSA(Function &F) {
DominatorTree &DT = getAnalysis<DominatorTreeWrapperPass>(F).getDomTree();
DT.recalculate(F); // CFG has been changed
- SSAUpdater SSA;
+ SSAUpdaterBulk SSA;
for (BasicBlock &BB : F) {
for (Instruction &I : BB) {
- SSA.Initialize(I.getType(), I.getName());
- SSA.AddAvailableValue(&BB, &I);
- for (auto UI = I.use_begin(), UE = I.use_end(); UI != UE;) {
- Use &U = *UI;
- ++UI;
+ unsigned VarID = SSA.AddVariable(I.getName(), I.getType());
+ SSA.AddAvailableValue(VarID, &BB, &I);
+ for (auto &U : I.uses()) {
auto *User = cast<Instruction>(U.getUser());
if (auto *UserPN = dyn_cast<PHINode>(User))
if (UserPN->getIncomingBlock(U) == &BB)
continue;
-
if (DT.dominates(&I, User))
continue;
- SSA.RewriteUseAfterInsertions(U);
+ SSA.AddUse(VarID, &U);
}
}
}
+ SSA.RewriteAllUses(&DT);
}
// Replace uses of longjmp with emscripten_longjmp. emscripten_longjmp takes
@@ -1301,24 +1300,14 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runSjLjOnFunction(Function &F) {
for (Instruction *I : SetjmpTableSizeInsts)
SetjmpTableSizeSSA.AddAvailableValue(I->getParent(), I);
- for (auto UI = SetjmpTable->use_begin(), UE = SetjmpTable->use_end();
- UI != UE;) {
- // Grab the use before incrementing the iterator.
- Use &U = *UI;
- // Increment the iterator before removing the use from the list.
- ++UI;
+ for (auto &U : make_early_inc_range(SetjmpTable->uses()))
if (auto *I = dyn_cast<Instruction>(U.getUser()))
if (I->getParent() != Entry)
SetjmpTableSSA.RewriteUse(U);
- }
- for (auto UI = SetjmpTableSize->use_begin(), UE = SetjmpTableSize->use_end();
- UI != UE;) {
- Use &U = *UI;
- ++UI;
+ for (auto &U : make_early_inc_range(SetjmpTableSize->uses()))
if (auto *I = dyn_cast<Instruction>(U.getUser()))
if (I->getParent() != Entry)
SetjmpTableSizeSSA.RewriteUse(U);
- }
// Finally, our modifications to the cfg can break dominance of SSA variables.
// For example, in this code,
diff --git a/llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll b/llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll
index a3bd17bff7e8..7ed2f21e2d71 100644
--- a/llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll
+++ b/llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll
@@ -142,7 +142,7 @@ if.then: ; preds = %entry
; CHECK-NEXT: %[[SETJMP_TABLE_SIZE1:.*]] = call i32 @getTempRet0()
; CHECK: if.then.split:
-; CHECK: %[[VAR1:.*]] = phi i32 [ %[[VAR0]], %if.then ], [ %[[VAR2:.*]], %if.end3 ]
+; CHECK: %[[VAR1:.*]] = phi i32 [ %[[VAR2:.*]], %if.end3 ], [ %[[VAR0]], %if.then ]
; CHECK: %[[SETJMP_TABLE_SIZE2:.*]] = phi i32 [ %[[SETJMP_TABLE_SIZE1]], %if.then ], [ %[[SETJMP_TABLE_SIZE3:.*]], %if.end3 ]
; CHECK: %[[SETJMP_TABLE2:.*]] = phi i32* [ %[[SETJMP_TABLE1]], %if.then ], [ %[[SETJMP_TABLE3:.*]], %if.end3 ]
; CHECK: store i32 %[[VAR1]], i32* @global_var, align 4
@@ -222,7 +222,7 @@ for.inc: ; preds = %for.cond
br label %for.cond
; CHECK: for.inc.split:
- ; CHECK: %var[[VARNO]] = phi i32 [ %var, %for.inc ]
+ ; CHECK: %var[[VARNO]] = phi i32 [ undef, %if.end ], [ %var, %for.inc ]
}
; Tests cases where longjmp function pointer is used in other ways than direct
More information about the llvm-commits
mailing list