[WebAssembly] Fix register use-def in FixIrreducibleControlFlow

FixIrreducibleControlFlow pass adds dispatch blocks with a `br_table`
that has multiple predecessors and successors, because it serves as
something like a traffic hub for BBs. As a result of this, there can be
register uses that are not dominated by a def in every path from the
entry block. For example, suppose register %a is defined in BB1 and used
in BB2, and there is a single path from BB1 and BB2:
```
BB1 -> ... -> BB2
```
After FixIrreducibleControlFlow runs, there can be a dispatch block
between these two BBs:
```
BB1 -> ... -> Dispatch -> ... -> BB2
```
And this dispatch block has multiple predecessors, now
there is a path to BB2 that does not first visit BB1, and in that path
%a is not dominated by a def anymore.

To fix this problem, we have been adding `IMPLICIT_DEF`s to all
registers in PrepareForLiveInternals pass, and then remove unnecessary
ones in OptimizeLiveIntervals pass after computing `LiveIntervals`. But
FixIrreducibleControlFlow pass itself ends up violating register use-def
relationship, resulting in invalid code. This was OK so far because
MIR verifier apparently didn't check this in validation. But @arsenm
fixed this and it caught this bug in validation
(https://github.com/llvm/llvm-project/issues/55249).

This CL moves the `IMPLICIT_DEF` adding routine from
PrepareForLiveInternals to FixIrreducibleControlFlow. We only run it
when FixIrreducibleControlFlow changes the code. And then
PrepareForLiveInternals doesn't do anything other than setting
`TracksLiveness` property, which is a prerequisite for running
`LiveIntervals` analysis, which is required by the next pass
OptimizeLiveIntervals.

But in our backend we don't seem to do anything that invalidates this up
until OptimizeLiveIntervals, and I'm not sure why we are calling
`invalidateLiveness` in ReplacePhysRegs pass, because what that pass
does is to replace physical registers with virtual ones 1-to-1. I
deleted the `invalidateLiveness` call there and we don't need to set
that flag explicitly, which obviates all the need for
PrepareForLiveInternals.

(By the way, This 'Liveness' here is different from `LiveIntervals`
analysis. Setting this only means BBs' live-in info is correct, all uses
are dominated by defs, `kill` flag is conservatively correct, which
means if there is a `kill` flag set it should be the last use. See
2a0837aab1/llvm/include/llvm/CodeGen/MachineFunction.h (L125-L134)
for details.)

So this CL removes PrepareForLiveInternals pass altogether. Something
similar to this was attempted by D56091 long ago but that came short of
actually removing the pass, and I couldn't land it because
FixIrreducibleControlFlow violated use-def relationship, which this CL
fixes.

This doesn't change output in any meaningful way. All test changes
except `irreducible-cfg.mir` are register numbering.

Also this will likely to reduce compilation time, because we have been
adding `IMPLICIT_DEF` for all registers every time `-O2` is given, but
now we do that only when there is irreducible control flow, which is
rare.

Fixes https://github.com/llvm/llvm-project/issues/55249.

Reviewed By: dschuff, kripken

Differential Revision: https://reviews.llvm.org/D125515

llvm/lib/Target/WebAssembly/CMakeLists.txt

@@ -43,7 +43,6 @@ add_llvm_target(WebAssemblyCodeGen
   WebAssemblyOptimizeLiveIntervals.cpp
   WebAssemblyOptimizeReturned.cpp
   WebAssemblyPeephole.cpp
-  WebAssemblyPrepareForLiveIntervals.cpp
   WebAssemblyRegisterInfo.cpp
   WebAssemblyRegColoring.cpp
   WebAssemblyRegNumbering.cpp

llvm/lib/Target/WebAssembly/WebAssembly.h

@@ -40,7 +40,6 @@ FunctionPass *createWebAssemblySetP2AlignOperands();
 // Late passes.
 FunctionPass *createWebAssemblyReplacePhysRegs();
 FunctionPass *createWebAssemblyNullifyDebugValueLists();
-FunctionPass *createWebAssemblyPrepareForLiveIntervals();
 FunctionPass *createWebAssemblyOptimizeLiveIntervals();
 FunctionPass *createWebAssemblyMemIntrinsicResults();
 FunctionPass *createWebAssemblyRegStackify();
@@ -66,7 +65,6 @@ void initializeWebAssemblyArgumentMovePass(PassRegistry &);
 void initializeWebAssemblySetP2AlignOperandsPass(PassRegistry &);
 void initializeWebAssemblyReplacePhysRegsPass(PassRegistry &);
 void initializeWebAssemblyNullifyDebugValueListsPass(PassRegistry &);
-void initializeWebAssemblyPrepareForLiveIntervalsPass(PassRegistry &);
 void initializeWebAssemblyOptimizeLiveIntervalsPass(PassRegistry &);
 void initializeWebAssemblyMemIntrinsicResultsPass(PassRegistry &);
 void initializeWebAssemblyRegStackifyPass(PassRegistry &);

llvm/lib/Target/WebAssembly/WebAssemblyFixIrreducibleControlFlow.cpp

@@ -492,6 +492,46 @@ FunctionPass *llvm::createWebAssemblyFixIrreducibleControlFlow() {
   return new WebAssemblyFixIrreducibleControlFlow();
 }
 
+// Test whether the given register has an ARGUMENT def.
+static bool hasArgumentDef(unsigned Reg, const MachineRegisterInfo &MRI) {
+  for (const auto &Def : MRI.def_instructions(Reg))
+    if (WebAssembly::isArgument(Def.getOpcode()))
+      return true;
+  return false;
+}
+
+// Add a register definition with IMPLICIT_DEFs for every register to cover for
+// register uses that don't have defs in every possible path.
+// TODO: This is fairly heavy-handed; find a better approach.
+static void addImplicitDefs(MachineFunction &MF) {
+  const MachineRegisterInfo &MRI = MF.getRegInfo();
+  const auto &TII = *MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
+  MachineBasicBlock &Entry = *MF.begin();
+  for (unsigned I = 0, E = MRI.getNumVirtRegs(); I < E; ++I) {
+    Register Reg = Register::index2VirtReg(I);
+
+    // Skip unused registers.
+    if (MRI.use_nodbg_empty(Reg))
+      continue;
+
+    // Skip registers that have an ARGUMENT definition.
+    if (hasArgumentDef(Reg, MRI))
+      continue;
+
+    BuildMI(Entry, Entry.begin(), DebugLoc(),
+            TII.get(WebAssembly::IMPLICIT_DEF), Reg);
+  }
+
+  // Move ARGUMENT_* instructions to the top of the entry block, so that their
+  // liveness reflects the fact that these really are live-in values.
+  for (MachineInstr &MI : llvm::make_early_inc_range(Entry)) {
+    if (WebAssembly::isArgument(MI.getOpcode())) {
+      MI.removeFromParent();
+      Entry.insert(Entry.begin(), &MI);
+    }
+  }
+}
+
 bool WebAssemblyFixIrreducibleControlFlow::runOnMachineFunction(
     MachineFunction &MF) {
   LLVM_DEBUG(dbgs() << "********** Fixing Irreducible Control Flow **********\n"
@@ -506,8 +546,15 @@ bool WebAssemblyFixIrreducibleControlFlow::runOnMachineFunction(
 
   if (LLVM_UNLIKELY(processRegion(&*MF.begin(), AllBlocks, MF))) {
     // We rewrote part of the function; recompute relevant things.
-    MF.getRegInfo().invalidateLiveness();
     MF.RenumberBlocks();
+    // Now we've inserted dispatch blocks, some register uses can have incoming
+    // paths without a def. For example, before this pass register %a was
+    // defined in BB1 and used in BB2, and there was only one path from BB1 and
+    // BB2. But if this pass inserts a dispatch block having multiple
+    // predecessors between the two BBs, now there are paths to BB2 without
+    // visiting BB1, and %a's use in BB2 is not dominated by its def. Adding
+    // IMPLICIT_DEFs to all regs is one simple way to fix it.
+    addImplicitDefs(MF);
     return true;
   }
 

llvm/lib/Target/WebAssembly/WebAssemblyOptimizeLiveIntervals.cpp

@@ -49,6 +49,11 @@ class WebAssemblyOptimizeLiveIntervals final : public MachineFunctionPass {
     MachineFunctionPass::getAnalysisUsage(AU);
   }
 
+  MachineFunctionProperties getRequiredProperties() const override {
+    return MachineFunctionProperties().set(
+        MachineFunctionProperties::Property::TracksLiveness);
+  }
+
   bool runOnMachineFunction(MachineFunction &MF) override;
 
 public:
@@ -102,7 +107,7 @@ bool WebAssemblyOptimizeLiveIntervals::runOnMachineFunction(
     SplitLIs.clear();
   }
 
-  // In PrepareForLiveIntervals, we conservatively inserted IMPLICIT_DEF
+  // In FixIrreducibleControlFlow, we conservatively inserted IMPLICIT_DEF
   // instructions to satisfy LiveIntervals' requirement that all uses be
   // dominated by defs. Now that LiveIntervals has computed which of these
   // defs are actually needed and which are dead, remove the dead ones.

llvm/lib/Target/WebAssembly/WebAssemblyPrepareForLiveIntervals.cpp

@@ -1,126 +0,0 @@
-//===- WebAssemblyPrepareForLiveIntervals.cpp - Prepare for LiveIntervals -===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-///
-/// \file
-/// Fix up code to meet LiveInterval's requirements.
-///
-/// Some CodeGen passes don't preserve LiveInterval's requirements, because
-/// they run after register allocation and it isn't important. However,
-/// WebAssembly runs LiveIntervals in a late pass. This pass transforms code
-/// to meet LiveIntervals' requirements; primarily, it ensures that all
-/// virtual register uses have definitions (IMPLICIT_DEF definitions if
-/// nothing else).
-///
-//===----------------------------------------------------------------------===//
-
-#include "MCTargetDesc/WebAssemblyMCTargetDesc.h"
-#include "Utils/WebAssemblyUtilities.h"
-#include "WebAssembly.h"
-#include "WebAssemblyMachineFunctionInfo.h"
-#include "WebAssemblySubtarget.h"
-#include "llvm/CodeGen/MachineFunctionPass.h"
-#include "llvm/CodeGen/MachineInstrBuilder.h"
-#include "llvm/CodeGen/MachineRegisterInfo.h"
-#include "llvm/CodeGen/Passes.h"
-#include "llvm/Support/Debug.h"
-#include "llvm/Support/raw_ostream.h"
-using namespace llvm;
-
-#define DEBUG_TYPE "wasm-prepare-for-live-intervals"
-
-namespace {
-class WebAssemblyPrepareForLiveIntervals final : public MachineFunctionPass {
-public:
-  static char ID; // Pass identification, replacement for typeid
-  WebAssemblyPrepareForLiveIntervals() : MachineFunctionPass(ID) {}
-
-private:
-  StringRef getPassName() const override {
-    return "WebAssembly Prepare For LiveIntervals";
-  }
-
-  void getAnalysisUsage(AnalysisUsage &AU) const override {
-    AU.setPreservesCFG();
-    MachineFunctionPass::getAnalysisUsage(AU);
-  }
-
-  bool runOnMachineFunction(MachineFunction &MF) override;
-};
-} // end anonymous namespace
-
-char WebAssemblyPrepareForLiveIntervals::ID = 0;
-INITIALIZE_PASS(WebAssemblyPrepareForLiveIntervals, DEBUG_TYPE,
-                "Fix up code for LiveIntervals", false, false)
-
-FunctionPass *llvm::createWebAssemblyPrepareForLiveIntervals() {
-  return new WebAssemblyPrepareForLiveIntervals();
-}
-
-// Test whether the given register has an ARGUMENT def.
-static bool hasArgumentDef(unsigned Reg, const MachineRegisterInfo &MRI) {
-  for (const auto &Def : MRI.def_instructions(Reg))
-    if (WebAssembly::isArgument(Def.getOpcode()))
-      return true;
-  return false;
-}
-
-bool WebAssemblyPrepareForLiveIntervals::runOnMachineFunction(
-    MachineFunction &MF) {
-  LLVM_DEBUG({
-    dbgs() << "********** Prepare For LiveIntervals **********\n"
-           << "********** Function: " << MF.getName() << '\n';
-  });
-
-  bool Changed = false;
-  MachineRegisterInfo &MRI = MF.getRegInfo();
-  const auto &TII = *MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
-  MachineBasicBlock &Entry = *MF.begin();
-
-  assert(!mustPreserveAnalysisID(LiveIntervalsID) &&
-         "LiveIntervals shouldn't be active yet!");
-
-  // We don't preserve SSA form.
-  MRI.leaveSSA();
-
-  // BranchFolding and perhaps other passes don't preserve IMPLICIT_DEF
-  // instructions. LiveIntervals requires that all paths to virtual register
-  // uses provide a definition. Insert IMPLICIT_DEFs in the entry block to
-  // conservatively satisfy this.
-  //
-  // TODO: This is fairly heavy-handed; find a better approach.
-  //
-  for (unsigned I = 0, E = MRI.getNumVirtRegs(); I < E; ++I) {
-    Register Reg = Register::index2VirtReg(I);
-
-    // Skip unused registers.
-    if (MRI.use_nodbg_empty(Reg))
-      continue;
-
-    // Skip registers that have an ARGUMENT definition.
-    if (hasArgumentDef(Reg, MRI))
-      continue;
-
-    BuildMI(Entry, Entry.begin(), DebugLoc(),
-            TII.get(WebAssembly::IMPLICIT_DEF), Reg);
-    Changed = true;
-  }
-
-  // Move ARGUMENT_* instructions to the top of the entry block, so that their
-  // liveness reflects the fact that these really are live-in values.
-  for (MachineInstr &MI : llvm::make_early_inc_range(Entry)) {
-    if (WebAssembly::isArgument(MI.getOpcode())) {
-      MI.removeFromParent();
-      Entry.insert(Entry.begin(), &MI);
-    }
-  }
-
-  // Ok, we're now ready to run the LiveIntervals analysis again.
-  MF.getProperties().set(MachineFunctionProperties::Property::TracksLiveness);
-
-  return Changed;
-}

llvm/lib/Target/WebAssembly/WebAssemblyReplacePhysRegs.cpp

@@ -72,9 +72,6 @@ bool WebAssemblyReplacePhysRegs::runOnMachineFunction(MachineFunction &MF) {
 
   assert(!mustPreserveAnalysisID(LiveIntervalsID) &&
          "LiveIntervals shouldn't be active yet!");
-  // We don't preserve SSA or liveness.
-  MRI.leaveSSA();
-  MRI.invalidateLiveness();
 
   for (unsigned PReg = WebAssembly::NoRegister + 1;
        PReg < WebAssembly::NUM_TARGET_REGS; ++PReg) {

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

@@ -63,7 +63,6 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeWebAssemblyTarget() {
   initializeWebAssemblyArgumentMovePass(PR);
   initializeWebAssemblySetP2AlignOperandsPass(PR);
   initializeWebAssemblyReplacePhysRegsPass(PR);
-  initializeWebAssemblyPrepareForLiveIntervalsPass(PR);
   initializeWebAssemblyOptimizeLiveIntervalsPass(PR);
   initializeWebAssemblyMemIntrinsicResultsPass(PR);
   initializeWebAssemblyRegStackifyPass(PR);
@@ -523,9 +522,6 @@ void WebAssemblyPassConfig::addPreEmitPass() {
 
   // Preparations and optimizations related to register stackification.
   if (getOptLevel() != CodeGenOpt::None) {
-    // LiveIntervals isn't commonly run this late. Re-establish preconditions.
-    addPass(createWebAssemblyPrepareForLiveIntervals());
-
     // Depend on LiveIntervals and perform some optimizations on it.
     addPass(createWebAssemblyOptimizeLiveIntervals());
 

llvm/test/CodeGen/WebAssembly/irreducible-cfg.mir

@@ -32,6 +32,8 @@ body: |
     %0:i32 = CONST_I32 100, implicit-def $arguments
     BR_IF %bb.2, %0:i32, implicit-def $arguments
   ; CHECK: bb.0.pred0:
+    ; CHECK:      %1:i32 = IMPLICIT_DEF
+    ; CHECK-NEXT: %0:i32 = IMPLICIT_DEF
     ; CHECK:      BR_IF %bb.2, %0, implicit-def $arguments
 
   bb.1.pred1:

llvm/test/CodeGen/WebAssembly/reg-stackify.ll

@@ -331,9 +331,9 @@ entry:
 ; NOREGS-NEXT:  local.get 1{{$}}
 ; NOREGS-NEXT:  local.get 0{{$}}
 ; NOREGS-NEXT:  i32.mul
-; NOREGS-NEXT:  local.tee   1{{$}}
+; NOREGS-NEXT:  local.tee   0{{$}}
 ; NOREGS-NEXT:  call        use_a{{$}}
-; NOREGS-NEXT:  local.get   1{{$}}
+; NOREGS-NEXT:  local.get   0{{$}}
 ; NOREGS-NEXT:  call        use_b{{$}}
 ; NOREGS-NEXT:  return{{$}}
 declare void @use_a(i32)
@@ -358,8 +358,8 @@ define void @simple_multiple_use(i32 %x, i32 %y) {
 ; NOREGS-NEXT:  local.get 1{{$}}
 ; NOREGS-NEXT:  local.get 0{{$}}
 ; NOREGS-NEXT:  i32.mul
-; NOREGS-NEXT:  local.tee   1{{$}}
-; NOREGS-NEXT:  local.get   1{{$}}
+; NOREGS-NEXT:  local.tee   0{{$}}
+; NOREGS-NEXT:  local.get   0{{$}}
 ; NOREGS-NEXT:  call        use_2{{$}}
 ; NOREGS-NEXT:  return{{$}}
 declare void @use_2(i32, i32)

llvm/test/CodeGen/WebAssembly/umulo-128-legalisation-lowering.ll

@@ -39,14 +39,14 @@ define { i128, i8 } @muloti_test(i128 %l, i128 %r) unnamed_addr #0 {
 ; WASM32: i32.const       $push5=, 40
 ; WASM32: i32.add         $push6=, $pop52, $pop5
 ; WASM32: i64.load        $push33=, 0($pop6)
-; WASM32: local.tee       $push32=, 1, $pop33
+; WASM32: local.tee       $push32=, 3, $pop33
 ; WASM32: local.get       $push53=, 5
 ; WASM32: i64.load        $push3=, 0($pop53)
 ; WASM32: local.get       $push54=, 5
 ; WASM32: i64.load        $push2=, 16($pop54)
 ; WASM32: i64.add         $push4=, $pop3, $pop2
 ; WASM32: i64.add         $push31=, $pop32, $pop4
-; WASM32: local.tee       $push30=, 3, $pop31
+; WASM32: local.tee       $push30=, 1, $pop31
 ; WASM32: i64.store       8($pop55), $pop30
 ; WASM32: local.get       $push62=, 0
 ; WASM32: local.get       $push56=, 2
@@ -66,8 +66,8 @@ define { i128, i8 } @muloti_test(i128 %l, i128 %r) unnamed_addr #0 {
 ; WASM32: i64.const       $push26=, 0
 ; WASM32: i64.ne          $push14=, $pop13, $pop26
 ; WASM32: i32.or          $push15=, $pop12, $pop14
-; WASM32: local.get       $push61=, 3
-; WASM32: local.get       $push60=, 1
+; WASM32: local.get       $push61=, 1
+; WASM32: local.get       $push60=, 3
 ; WASM32: i64.lt_u        $push16=, $pop61, $pop60
 ; WASM32: i32.or          $push17=, $pop15, $pop16
 ; WASM32: i32.store8      16($pop62), $pop17

llvm/test/CodeGen/WebAssembly/umulo-i64.ll

@@ -21,7 +21,7 @@ attributes #1 = { nounwind readnone speculatable }
 
 ; CHECK-LABEL: wut:
 ; CHECK: call     __multi3, $2, $0, $pop0, $1, $pop7
-; CHECK: i64.load $0=, 8($2)
+; CHECK: i64.load $1=, 8($2)
 define i1 @wut(i64, i64) {
 start:
   %2 = call { i64, i1 } @llvm.umul.with.overflow.i64(i64 %0, i64 %1)

llvm/utils/gn/secondary/llvm/lib/Target/WebAssembly/BUILD.gn

@@ -60,7 +60,6 @@ static_library("LLVMWebAssemblyCodeGen") {
     "WebAssemblyOptimizeLiveIntervals.cpp",
     "WebAssemblyOptimizeReturned.cpp",
     "WebAssemblyPeephole.cpp",
-    "WebAssemblyPrepareForLiveIntervals.cpp",
     "WebAssemblyRegColoring.cpp",
     "WebAssemblyRegNumbering.cpp",
     "WebAssemblyRegStackify.cpp",