[llvm] 473ef10 - [WebAssembly] Demote PHIs in catchswitch BB only (#81570)

[via llvm-commits]

Author: Heejin Ahn
Date: 2024-02-13T13:43:21-08:00
New Revision: 473ef10b0fc93eeb2cbb3b2cf2f1b748eac6ddd9

URL: https://github.com/llvm/llvm-project/commit/473ef10b0fc93eeb2cbb3b2cf2f1b748eac6ddd9
DIFF: https://github.com/llvm/llvm-project/commit/473ef10b0fc93eeb2cbb3b2cf2f1b748eac6ddd9.diff

LOG: [WebAssembly] Demote PHIs in catchswitch BB only (#81570)

`DemoteCatchSwitchPHIOnly` option in `WinEHPrepare` pass was added in
https://github.com/llvm/llvm-project/commit/99d60e0dabcf20f4db683da83cde905b7a1373de,
because Wasm EH uses `WinEHPrepare`, but it doesn't need to demote all
PHIs. PHIs in `catchswitch` BBs have to be removed (= demoted) because
`catchswitch`s are removed in ISel and `catchswitch` BBs are removed as
well, so they can't have other instructions.

But because Wasm EH doesn't use funclets, so PHIs in `catchpad` or
`cleanuppad` BBs don't need to be demoted. That was the reason
`DemoteCatchSwitchPHIOnly` option was added, in order not to demote more
instructions unnecessarily.

The problem is it should have been set to `true` for Wasm EH. (Its
default value is `false` for WinEH) And I mistakenly set it to `false`
and wasn't aware about this for more than 5 years. This was not the end
of the world; it just means we've been demoting more instructions than
we should, possibly huting code size. In practice I think it would've
had hardly any effect in real performance given that the occurrence of
PHIs in `catchpad` or `cleanuppad` BBs are not very frequent and many
people run other optimizers like Binaryen anyway.

Added: 
    

Modified: 
    llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
    llvm/lib/CodeGen/TargetPassConfig.cpp
    llvm/test/CodeGen/WebAssembly/wasm-eh-prepare.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
index 6cf54085915230..4172fbc96d1e5d 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
@@ -249,7 +249,8 @@ void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf,
                "WinEHPrepare failed to remove PHIs from imaginary BBs");
         continue;
       }
-      if (isa<FuncletPadInst>(PadInst))
+      if (isa<FuncletPadInst>(PadInst) &&
+          Personality != EHPersonality::Wasm_CXX)
         assert(&*BB.begin() == PadInst && "WinEHPrepare failed to demote PHIs");
     }
 

diff  --git a/llvm/lib/CodeGen/TargetPassConfig.cpp b/llvm/lib/CodeGen/TargetPassConfig.cpp
index e82f14e878141a..2ed39a5696e205 100644
--- a/llvm/lib/CodeGen/TargetPassConfig.cpp
+++ b/llvm/lib/CodeGen/TargetPassConfig.cpp
@@ -918,7 +918,7 @@ void TargetPassConfig::addPassesToHandleExceptions() {
     // on catchpads and cleanuppads because it does not outline them into
     // funclets. Catchswitch blocks are not lowered in SelectionDAG, so we
     // should remove PHIs there.
-    addPass(createWinEHPass(/*DemoteCatchSwitchPHIOnly=*/false));
+    addPass(createWinEHPass(/*DemoteCatchSwitchPHIOnly=*/true));
     addPass(createWasmEHPass());
     break;
   case ExceptionHandling::None:

diff  --git a/llvm/test/CodeGen/WebAssembly/wasm-eh-prepare.ll b/llvm/test/CodeGen/WebAssembly/wasm-eh-prepare.ll
index bd577e387c72b7..164c138cb7578e 100644
--- a/llvm/test/CodeGen/WebAssembly/wasm-eh-prepare.ll
+++ b/llvm/test/CodeGen/WebAssembly/wasm-eh-prepare.ll
@@ -2,6 +2,7 @@
 ; RUN: opt < %s -win-eh-prepare -demote-catchswitch-only -wasm-eh-prepare -S --mattr=+atomics,+bulk-memory | FileCheck %s
 ; RUN: opt < %s -passes='win-eh-prepare<demote-catchswitch-only>,wasm-eh-prepare' -S | FileCheck %s
 ; RUN: opt < %s -passes='win-eh-prepare<demote-catchswitch-only>,wasm-eh-prepare' -S --mattr=+atomics,+bulk-memory | FileCheck %s
+; RUN: llc < %s -wasm-enable-eh -exception-model=wasm -mattr=+exception-handling -stop-after=wasm-eh-prepare | FileCheck %s
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
@@ -245,7 +246,6 @@ bb.true:                                          ; preds = %entry
 bb.true.0:                                        ; preds = %bb.true
   br label %merge
 
-; CHECK:      bb.false
 bb.false:                                         ; preds = %entry
   br label %merge