[WebAssembly] Add NullifyDebugValueLists pass

[aheejin]

Summary

WebAssemblyDebugValueManager does not currently handle
DBG_VALUE_LIST, which is a recent addition to LLVM. We tried to
nullify them within the constructor of WebAssemblyDebugValueManager in
D102589, but it made the class error-prone to use because it deletes
instructions within the constructor and thus invalidates existing
iterators within the BB, so the user of the class should take special
care not to use invalidated iterators. This actually caused a bug in
ExplicitLocals pass.

Instead of trying to fix ExplicitLocals pass to make the iterator usage
correct, which is possible but error-prone, this adds
NullifyDebugValueLists pass that nullifies all DBG_VALUE_LIST
instructions before we run WebAssembly specific passes in the backend.
We can remove this pass after we implement handlers for
DBG_VALUE_LISTs in WebAssemblyDebugValueManager and elsewhere.

Fixes https://github.com/emscripten-core/emscripten/issues/14255.

[dschuff]

llvm/test/DebugInfo/WebAssembly/dbg-value-list.ll

12

Would it be possible to check that there actually are DBG_VALUE_LISTs in the input to the MIR pipeline (perhaps using -stop-after or something)? It's not obvious (to me) how the IR below leads to DBG_VALUE_LIST being created and it seems unlikely to be guaranteed.

[aheejin]

This also contains a revert of D102589.

[aheejin]

Add BEFORE and AFTER tests

llvm/test/DebugInfo/WebAssembly/dbg-value-list.ll

12

The presence of DIArgList in the first operand of llvm.dbg.value generates DBG_VALUE_LISTs. I added BEFORE and AFTER checking in the test.

[aheejin]

Add comment for DIArgList

llvm/test/DebugInfo/WebAssembly/dbg-value-list.ll

12

Also added a comment explaining DIArgList creates DBG_VALUE_LIST

[aheejin]

Better comment

[aheejin]

More

llvm/lib/Target/WebAssembly/CMakeLists.txt

add_llvm_target(WebAssemblyCodeGen
   WebAssemblyISelDAGToDAG.cpp
   WebAssemblyISelLowering.cpp
   WebAssemblyInstrInfo.cpp
   WebAssemblyLowerBrUnless.cpp
   WebAssemblyLowerEmscriptenEHSjLj.cpp
   WebAssemblyLowerGlobalDtors.cpp
   WebAssemblyMachineFunctionInfo.cpp
   WebAssemblyMCInstLower.cpp
+  WebAssemblyNullifyDebugValueLists.cpp
   WebAssemblyOptimizeLiveIntervals.cpp
   WebAssemblyOptimizeReturned.cpp
   WebAssemblyPeephole.cpp
   WebAssemblyPrepareForLiveIntervals.cpp
   WebAssemblyRegisterInfo.cpp
   WebAssemblyRegColoring.cpp
   WebAssemblyRegNumbering.cpp
   WebAssemblyRegStackify.cpp

llvm/lib/Target/WebAssembly/WebAssembly.h

 // ISel and immediate followup passes.
 FunctionPass *createWebAssemblyISelDag(WebAssemblyTargetMachine &TM,
                                        CodeGenOpt::Level OptLevel);
 FunctionPass *createWebAssemblyArgumentMove();
 FunctionPass *createWebAssemblySetP2AlignOperands();
 
 // Late passes.
 FunctionPass *createWebAssemblyReplacePhysRegs();
+FunctionPass *createWebAssemblyNullifyDebugValueLists();
 FunctionPass *createWebAssemblyPrepareForLiveIntervals();
 FunctionPass *createWebAssemblyOptimizeLiveIntervals();
 FunctionPass *createWebAssemblyMemIntrinsicResults();
 FunctionPass *createWebAssemblyRegStackify();
 FunctionPass *createWebAssemblyRegColoring();
 FunctionPass *createWebAssemblyFixBrTableDefaults();
 FunctionPass *createWebAssemblyFixIrreducibleControlFlow();
 FunctionPass *createWebAssemblyLateEHPrepare();

 void initializeWebAssemblyAddMissingPrototypesPass(PassRegistry &);
 void initializeWebAssemblyLowerEmscriptenEHSjLjPass(PassRegistry &);
 void initializeLowerGlobalDtorsPass(PassRegistry &);
 void initializeFixFunctionBitcastsPass(PassRegistry &);
 void initializeOptimizeReturnedPass(PassRegistry &);
 void initializeWebAssemblyArgumentMovePass(PassRegistry &);
 void initializeWebAssemblySetP2AlignOperandsPass(PassRegistry &);
 void initializeWebAssemblyReplacePhysRegsPass(PassRegistry &);
+void initializeWebAssemblyNullifyDebugValueListsPass(PassRegistry &);
 void initializeWebAssemblyPrepareForLiveIntervalsPass(PassRegistry &);
 void initializeWebAssemblyOptimizeLiveIntervalsPass(PassRegistry &);
 void initializeWebAssemblyMemIntrinsicResultsPass(PassRegistry &);
 void initializeWebAssemblyRegStackifyPass(PassRegistry &);
 void initializeWebAssemblyRegColoringPass(PassRegistry &);
 void initializeWebAssemblyFixBrTableDefaultsPass(PassRegistry &);
 void initializeWebAssemblyFixIrreducibleControlFlowPass(PassRegistry &);
 void initializeWebAssemblyLateEHPreparePass(PassRegistry &);

llvm/lib/Target/WebAssembly/WebAssemblyDebugValueManager.cpp

 #include "WebAssembly.h"
 #include "WebAssemblyMachineFunctionInfo.h"
 #include "llvm/CodeGen/MachineInstr.h"
 
 using namespace llvm;
 
 WebAssemblyDebugValueManager::WebAssemblyDebugValueManager(
     MachineInstr *Instr) {
-  const auto *MF = Instr->getParent()->getParent();
-  const auto *TII = MF->getSubtarget<WebAssemblySubtarget>().getInstrInfo();
-
   // This code differs from MachineInstr::collectDebugValues in that it scans
   // the whole BB, not just contiguous DBG_VALUEs.
   if (!Instr->getOperand(0).isReg())
     return;
   CurrentReg = Instr->getOperand(0).getReg();
 
-  SmallVector<MachineInstr *, 2> DbgValueLists;
   MachineBasicBlock::iterator DI = *Instr;
   ++DI;
   for (MachineBasicBlock::iterator DE = Instr->getParent()->end(); DI != DE;
        ++DI) {
     if (DI->isDebugValue() &&
         DI->hasDebugOperandForReg(Instr->getOperand(0).getReg()))
-      DI->getOpcode() == TargetOpcode::DBG_VALUE
-          ? DbgValues.push_back(&*DI)
-          : DbgValueLists.push_back(&*DI);
-  }
-
-  // This class currently cannot handle DBG_VALUE_LISTs correctly. So this
-  // converts DBG_VALUE_LISTs to "DBG_VALUE $noreg", which will appear as
-  // "optimized out". This can invalidate existing iterators pointing to
-  // instructions within this BB from the caller.
-  // See https://bugs.llvm.org/show_bug.cgi?id=50361
-  // TODO Correctly handle DBG_VALUE_LISTs
-  for (auto *DVL : DbgValueLists) {
-    BuildMI(*DVL->getParent(), DVL, DVL->getDebugLoc(),
-            TII->get(TargetOpcode::DBG_VALUE), false, Register(),
-            DVL->getOperand(0).getMetadata(), DVL->getOperand(1).getMetadata());
-    DVL->eraseFromParent();
+      DbgValues.push_back(&*DI);
   }
 }
 
 void WebAssemblyDebugValueManager::move(MachineInstr *Insert) {
   MachineBasicBlock *MBB = Insert->getParent();
   for (MachineInstr *DBI : reverse(DbgValues))
     MBB->splice(Insert, DBI->getParent(), DBI);
 }

llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp

for (MachineBasicBlock::iterator I = MBB.begin(), E = MBB.end(); I != E;) {
       }
 
       // Insert local.sets for any defs that aren't stackified yet.
       for (auto &Def : MI.defs()) {
         Register OldReg = Def.getReg();
         if (!MFI.isVRegStackified(OldReg)) {
           const TargetRegisterClass *RC = MRI.getRegClass(OldReg);
           Register NewReg = MRI.createVirtualRegister(RC);
+          auto InsertPt = std::next(MI.getIterator());
           if (UseEmpty[Register::virtReg2Index(OldReg)]) {
             unsigned Opc = getDropOpcode(RC);
-            MachineInstr *Drop = BuildMI(MBB, std::next(MI.getIterator()),
-                                         MI.getDebugLoc(), TII->get(Opc))
-                                     .addReg(NewReg);
+            MachineInstr *Drop =
+                BuildMI(MBB, InsertPt, MI.getDebugLoc(), TII->get(Opc))
             // After the drop instruction, this reg operand will not be used
             Drop->getOperand(0).setIsKill();
             if (MFI.isFrameBaseVirtual() && OldReg == MFI.getFrameBaseVreg())
               MFI.clearFrameBaseVreg();
           } else {
             unsigned LocalId = getLocalId(Reg2Local, MFI, CurLocal, OldReg);
             unsigned Opc = getLocalSetOpcode(RC);
 
             WebAssemblyDebugValueManager(&MI).replaceWithLocal(LocalId);
 
-            BuildMI(MBB, std::next(MI.getIterator()), MI.getDebugLoc(),
-                    TII->get(Opc))
+            BuildMI(MBB, InsertPt, MI.getDebugLoc(), TII->get(Opc))
                 .addImm(LocalId)
                 .addReg(NewReg);
           }
           // This register operand of the original instruction is now being used
           // by the inserted drop or local.set instruction, so make it not dead
           // yet.
           Def.setReg(NewReg);
           Def.setIsDead(false);

llvm/lib/Target/WebAssembly/WebAssemblyNullifyDebugValueLists.cpp

+//=== WebAssemblyNullifyDebugValueLists.cpp - Nullify DBG_VALUE_LISTs   ---===//
+//
+// 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
+/// Nullify DBG_VALUE_LISTs instructions as a temporary measure before we
+/// implement DBG_VALUE_LIST handling in WebAssemblyDebugValueManager.
+/// See https://bugs.llvm.org/show_bug.cgi?id=50361.
+/// TODO Correctly handle DBG_VALUE_LISTs
+///
+//===----------------------------------------------------------------------===//
+
+#include "WebAssembly.h"
+#include "WebAssemblySubtarget.h"
+using namespace llvm;
+
+#define DEBUG_TYPE "wasm-nullify-dbg-value-lists"
+
+namespace {
+class WebAssemblyNullifyDebugValueLists final : public MachineFunctionPass {
+  StringRef getPassName() const override {
+    return "WebAssembly Nullify DBG_VALUE_LISTs";
+  }
+
+  bool runOnMachineFunction(MachineFunction &MF) override;
+
+public:
+  static char ID; // Pass identification, replacement for typeid
+  WebAssemblyNullifyDebugValueLists() : MachineFunctionPass(ID) {}
+};
+} // end anonymous namespace
+
+char WebAssemblyNullifyDebugValueLists::ID = 0;
+INITIALIZE_PASS(WebAssemblyNullifyDebugValueLists, DEBUG_TYPE,
+                "WebAssembly Nullify DBG_VALUE_LISTs", false, false)
+
+FunctionPass *llvm::createWebAssemblyNullifyDebugValueLists() {
+  return new WebAssemblyNullifyDebugValueLists();
+}
+
+bool WebAssemblyNullifyDebugValueLists::runOnMachineFunction(
+    MachineFunction &MF) {
+  LLVM_DEBUG(dbgs() << "********** Nullify DBG_VALUE_LISTs **********\n"
+                       "********** Function: "
+                    << MF.getName() << '\n');
+  const auto &TII = *MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
+  SmallVector<MachineInstr *, 2> DbgValueLists;
+  for (auto &MBB : MF)
+    for (auto &MI : MBB)
+      if (MI.getOpcode() == TargetOpcode::DBG_VALUE_LIST)
+        DbgValueLists.push_back(&MI);
+
+  // Our backend, including WebAssemblyDebugValueManager, currently cannot
+  // handle DBG_VALUE_LISTs correctly. So this converts DBG_VALUE_LISTs to
+  // "DBG_VALUE $noreg", which will appear as "optimized out".
+  for (auto *DVL : DbgValueLists) {
+    BuildMI(*DVL->getParent(), DVL, DVL->getDebugLoc(),
+            TII.get(TargetOpcode::DBG_VALUE), false, Register(),
+            DVL->getOperand(0).getMetadata(), DVL->getOperand(1).getMetadata());
+    DVL->eraseFromParent();
+  }
+
+  return !DbgValueLists.empty();
+}

llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeWebAssemblyTarget() {
   initializeWebAssemblyArgumentMovePass(PR);
   initializeWebAssemblySetP2AlignOperandsPass(PR);
   initializeWebAssemblyReplacePhysRegsPass(PR);
   initializeWebAssemblyPrepareForLiveIntervalsPass(PR);
   initializeWebAssemblyOptimizeLiveIntervalsPass(PR);
   initializeWebAssemblyMemIntrinsicResultsPass(PR);
   initializeWebAssemblyRegStackifyPass(PR);
   initializeWebAssemblyRegColoringPass(PR);
+  initializeWebAssemblyNullifyDebugValueListsPass(PR);
   initializeWebAssemblyFixIrreducibleControlFlowPass(PR);
   initializeWebAssemblyLateEHPreparePass(PR);
   initializeWebAssemblyExceptionInfoPass(PR);
   initializeWebAssemblyCFGSortPass(PR);
   initializeWebAssemblyCFGStackifyPass(PR);
   initializeWebAssemblyExplicitLocalsPass(PR);
   initializeWebAssemblyLowerBrUnlessPass(PR);
   initializeWebAssemblyRegNumberingPass(PR);
void WebAssemblyPassConfig::addPostRegAlloc() {
   disablePass(&MachineBlockPlacementID);
 
   TargetPassConfig::addPostRegAlloc();
 }
 
 void WebAssemblyPassConfig::addPreEmitPass() {
   TargetPassConfig::addPreEmitPass();
 
+  // Nullify DBG_VALUE_LISTs that we cannot handle.
+  addPass(createWebAssemblyNullifyDebugValueLists());
+
   // Eliminate multiple-entry loops.
   addPass(createWebAssemblyFixIrreducibleControlFlow());
 
   // Do various transformations for exception handling.
   // Every CFG-changing optimizations should come before this.
   if (TM->Options.ExceptionModel == ExceptionHandling::Wasm)
     addPass(createWebAssemblyLateEHPrepare());
 

llvm/test/CodeGen/WebAssembly/reg-stackify-dbg.mir

-# RUN: llc -mtriple=wasm32-unknown-unknown -run-pass wasm-reg-stackify %s -o - | FileCheck %s
-
---- |
-  target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-  target triple = "wasm32-unknown-unknown"
-
-  define void @dbg_value_list_test() {
-    ret void
-  }
-
-  !llvm.module.flags = !{!0}
-  !llvm.dbg.cu = !{!1}
-
-  !0 = !{i32 2, !"Debug Info Version", i32 3}
-  !1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang version 3.9.0 (trunk 266005) (llvm/trunk 266105)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !3)
-  !2 = !DIFile(filename: "test.c", directory: "/")
-  !3 = !{}
-  !4 = distinct !DISubprogram(name: "dbg_value_list_test", scope: !2, file: !2, line: 10, type: !5, isLocal: false, isDefinition: true, scopeLine: 11, flags: DIFlagPrototyped, isOptimized: true, unit: !1, retainedNodes: !3)
-  !5 = !DISubroutineType(types: !3)
-  !6 = !DILocalVariable(name: "var", scope: !4, file: !2, line: 15, type: !7)
-  !7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
-  !8 = !DILocation(line: 15, column: 6, scope: !4)
-...
-
-# WebAssemblyDebugValueManager currently does not handle DBG_VALUE_LIST
-# instructions correctly and instead effectively nullifying them by turning them
-# into "DBG_VALUE $noreg". See https://bugs.llvm.org/show_bug.cgi?id=50361.
-# (Otherwise DBG_VALUE_LIST instructions can be exponentially and possibly
-# incorrectly copied.)
-# This tests if DBG_VALUE_LIST is nullified as intended.
-
-# CHECK-LABEL: name: dbg_value_list_test
-name: dbg_value_list_test
-liveins:
-  - { reg: '$arguments' }
-body: |
-  bb.0:
-    ; CHECK: DBG_VALUE $noreg, $noreg
-    %0:i32 = ARGUMENT_i32 0, implicit $arguments
-    %1:i32 = ARGUMENT_i32 1, implicit $arguments
-    %2:i32 = ARGUMENT_i32 2, implicit $arguments
-    %3:i32 = LOAD_I32_A32 2, 0, %0:i32, implicit-def dead $arguments
-    %4:i32 = LT_U_I32 %3:i32, %1:i32, implicit-def dead $arguments
-    %5:i32 = GE_U_I32 %4:i32, %2:i32, implicit-def dead $arguments
-    %6:i32 = OR_I32 %5:i32, %4:i32, implicit-def dead $arguments
-    ; This should become "DBG_VALUE $noreg" and should not be copied when %4 is
-    ; tee'd
-    ; CHECK-NOT: DBG_VALUE_LIST
-    DBG_VALUE_LIST !6, !DIExpression(), %4:i32, debug-location !8
-    RETURN %6:i32, implicit-def dead $arguments
-...

llvm/test/DebugInfo/WebAssembly/dbg-value-list.ll

+; RUN: llc %s -stop-before wasm-nullify-dbg-value-lists -o - | FileCheck %s --check-prefix=BEFORE
+; RUN: llc %s -stop-after wasm-nullify-dbg-value-lists -o - | FileCheck %s --check-prefix=AFTER
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+; WebAssembly backend does not currently handle DBG_VALUE_LIST instructions
+; correctly. In the meantime, they are converted to 'DBG_VALUE $noreg's in
+; WebAssemblyNullifyDebugValueLists pass.
+
+; BEFORE: DBG_VALUE_LIST
+; AFTER-NOT: DBG_VALUE_LIST
+; AFTER: DBG_VALUE $noreg, $noreg
+define i32 @dbg_value_list_test() !dbg !6 {
+entry:
+  %0 = call i32 @foo(), !dbg !9
+  %1 = call i32 @foo(), !dbg !10
+  %2 = add i32 %0, %1, !dbg !11
+  ; This DIArgList operand generates a DBG_VALUE_LIST instruction
+  call void @llvm.dbg.value(metadata !DIArgList(i32 %0, i32 %1), metadata !8, metadata !DIExpression()), !dbg !11
+  ret i32 %2, !dbg !12
+}
+
+declare i32 @foo()
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git ed7aaf832444411ce93aa0443425ce401f5c7a8e)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "test.c", directory: "/home/llvm-project")
+!2 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = distinct !DISubprogram(name: "", scope: !1, file: !1, line: 3, type: !7, scopeLine: 3, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!7 = !DISubroutineType(types: !{null})
+!8 = !DILocalVariable(name: "i", scope: !6, file: !1, line: 4, type: !2)
+!9 = !DILocation(line: 4, column: 11, scope: !6)
+!10 = !DILocation(line: 5, column: 11, scope: !6)
+!11 = !DILocation(line: 6, column: 3, scope: !6)
+!12 = !DILocation(line: 7, column: 1, scope: !6)