From d5244fb16070068fc1dfae5804ab8ff86d25deb9 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Mon, 16 Aug 2021 23:30:02 -0700 Subject: [PATCH] [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 --- .../WebAssemblyLowerEmscriptenEHSjLj.cpp | 29 +++++++--------------- llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll | 4 +-- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp index 01aae06..3488ef4 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(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(U.getUser()); if (auto *UserPN = dyn_cast(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(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(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 a3bd17b..7ed2f21 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 -- 2.7.4