[WebAssembly] Preserve debug frame base information through register coloring

[dschuff]

Summary

2 fixes:

• Register coloring can re-assign virtual registers. When the frame base register is colored, update the DwarfFrameBase accordingly
• When the frame base register is stackified, do not attempt to encode DW_AT_frame_base as a local

In the future we will presumably want to handle this case better but for now we can emit worse debug info rather than crashing.

[merge_guards_bot]

Unit tests: fail. 62274 tests passed, 1 failed and 827 were skipped.

failed: Clang.CodeGenOpenCL/amdgpu-features.cl

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[dschuff]

• * When stackifying the frame base, unset frameBaseVreg

[kripken]

Makes sense to me, nice. Also verified no crashes on wasm0 and wasm3 on the emscripten test suite.

[dschuff]

• clang-format

[merge_guards_bot]

Unit tests: fail. 62275 tests passed, 1 failed and 827 were skipped.

failed: Clang.CodeGenOpenCL/amdgpu-features.cl

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[dschuff]

I'll go ahead and land this to reduce the number of cases where we crash; but I'd be interested in hearing if @yurydelendik has found any cases that were asserting before that this fixes, or if any of the wasm subscribers have other opinions.

[merge_guards_bot]

Unit tests: fail. 62275 tests passed, 1 failed and 827 were skipped.

failed: Clang.CodeGenOpenCL/amdgpu-features.cl

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[yurydelendik]

In D73581#1845884, @dschuff wrote:

I'll go ahead and land this to reduce the number of cases where we crash; but I'd be interested in hearing if @yurydelendik has found any cases that were asserting before that this fixes, or if any of the wasm subscribers have other opinions.

Disabling frame base register in some hard cases is the right approach. I did not see any such crashes/cases yet.

llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp

 /// Return a local id number for the given register, assigning it a new one
 /// if it doesn't yet have one.
 static unsigned getLocalId(DenseMap<unsigned, unsigned> &Reg2Local,
                            WebAssemblyFunctionInfo &MFI, unsigned &CurLocal,
                            unsigned Reg) {
   auto P = Reg2Local.insert(std::make_pair(Reg, CurLocal));
   if (P.second) {
     // Mark the local allocated for the frame base vreg.
-    if (MFI.isFrameBaseVirtual() && Reg == MFI.getFrameBaseVreg())
+    if (MFI.isFrameBaseVirtual() && Reg == MFI.getFrameBaseVreg()) {
+      LLVM_DEBUG({
+          dbgs() << "Allocating local " << CurLocal << "for VReg " << Register::virtReg2Index(Reg) << '\n';
+        });
       MFI.setFrameBaseLocal(CurLocal);
+    }
     ++CurLocal;
   }
   return P.first->second;
 }
 
 /// Get the appropriate drop opcode for the given register class.
 static unsigned getDropOpcode(const TargetRegisterClass *RC) {
   if (RC == &WebAssembly::I32RegClass)

llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp

void WebAssemblyFrameLowering::emitEpilogue(MachineFunction &MF,
   writeSPToGlobal(SPReg, MF, MBB, InsertPt, DL);
 }
 
 TargetFrameLowering::DwarfFrameBase
 WebAssemblyFrameLowering::getDwarfFrameBase(const MachineFunction &MF) const {
   DwarfFrameBase Loc;
   Loc.Kind = DwarfFrameBase::WasmFrameBase;
   const WebAssemblyFunctionInfo &MFI = *MF.getInfo<WebAssemblyFunctionInfo>();
-  if (needsSP(MF)) {
+  if (needsSP(MF) && MFI.isFrameBaseVirtual()) {
     unsigned LocalNum = MFI.getFrameBaseLocal();
     Loc.Location.WasmLoc = {WebAssembly::TI_LOCAL_START, LocalNum};
   } else {
     // TODO: This should work on a breakpoint at a function with no frame,
     // but probably won't work for traversing up the stack.
     // TODO: This needs a relocation for correct __stack_pointer
     Loc.Location.WasmLoc = {WebAssembly::TI_GLOBAL_START, 0};
   }
   return Loc;
 }

llvm/lib/Target/WebAssembly/WebAssemblyMachineFunctionInfo.h

unsigned getBasePointerVreg() const {
     assert(BasePtrVreg != -1U && "Base ptr vreg hasn't been set");
     return BasePtrVreg;
   }
   void setFrameBaseVreg(unsigned Reg) { FrameBaseVreg = Reg; }
   unsigned getFrameBaseVreg() const {
     assert(FrameBaseVreg != -1U && "Frame base vreg hasn't been set");
     return FrameBaseVreg;
   }
+  void clearFrameBaseVreg() {FrameBaseVreg = -1U;}
   // Return true if the frame base physreg has been replaced by a virtual reg.
   bool isFrameBaseVirtual() const { return FrameBaseVreg != -1U; }
   void setFrameBaseLocal(unsigned Local) { FrameBaseLocal = Local; }
   unsigned getFrameBaseLocal() const {
     assert(FrameBaseLocal != -1U && "Frame base local hasn't been set");
     return FrameBaseLocal;
   }
   void setBasePointerVreg(unsigned Reg) { BasePtrVreg = Reg; }

llvm/lib/Target/WebAssembly/WebAssemblyRegColoring.cpp

if (!MRI->isLiveIn(Old))
       continue_outer:;
       }
 
     unsigned New = SortedIntervals[Color]->reg;
     SlotMapping[I] = New;
     Changed |= Old != New;
     UsedColors.set(Color);
     Assignments[Color].push_back(LI);
+    // If we reassigned the stack pointer, update the debug frame base info.
+    if (Old != New && MFI.isFrameBaseVirtual() && MFI.getFrameBaseVreg() == Old)
+      MFI.setFrameBaseVreg(New);
     LLVM_DEBUG(dbgs() << "Assigning vreg" << Register::virtReg2Index(LI->reg)
                       << " to vreg" << Register::virtReg2Index(New) << "\n");
   }
   if (!Changed)
     return false;
 
   // Rewrite register operands.
   for (size_t I = 0, E = SortedIntervals.size(); I < E; ++I) {
     unsigned Old = SortedIntervals[I]->reg;
     unsigned New = SlotMapping[I];
     if (Old != New)
       MRI->replaceRegWith(Old, New);
   }
   return true;
 }

llvm/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

for (auto MII = MBB.rbegin(); MII != MBB.rend(); ++MII) {
         // this makes things simpler (LiveIntervals' handleMove function only
         // supports intra-block moves) and it's MachineSink's job to catch all
         // the sinking opportunities anyway.
         bool SameBlock = Def->getParent() == &MBB;
         bool CanMove = SameBlock && isSafeToMove(Def, Insert, AA, MRI) &&
                        !TreeWalker.isOnStack(Reg);
         if (CanMove && hasOneUse(Reg, Def, MRI, MDT, LIS)) {
           Insert = moveForSingleUse(Reg, Op, Def, MBB, Insert, LIS, MFI, MRI);
+
+          // If we are removing the frame base reg completely, remove the debug
+          // info as well.
+          // TODO: Encode this properly as a stackified value.
+          if (MFI.isFrameBaseVirtual() && MFI.getFrameBaseVreg() == Reg)
+            MFI.clearFrameBaseVreg();
         } else if (shouldRematerialize(*Def, AA, TII)) {
           Insert =
               rematerializeCheapDef(Reg, Op, *Def, MBB, Insert->getIterator(),
                                     LIS, MFI, MRI, TII, TRI);
         } else if (CanMove &&
                    oneUseDominatesOtherUses(Reg, Op, MBB, MRI, MDT, LIS, MFI)) {
           Insert = moveAndTeeForMultiUse(Reg, Op, Def, MBB, Insert, LIS, MFI,
                                          MRI, TII);

llvm/test/CodeGen/WebAssembly/debugtest-opt.ll

+; RUN: llc < %s -filetype=obj -o - | llvm-dwarfdump -
+
+; To regenerate this file, use approximately the following C code:
+; int* globl;
+; void baz(int arg) {
+;   int locl;
+;   globl = &locl;
+; }
+
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT:                DW_AT_low_pc
+; CHECK-NEXT:                DW_AT_high_pc
+;; Check that we fall back to the default frame base (the global)
+; CHECK-NEXT:                DW_AT_frame_base (DW_OP_WASM_location 0x1 +0, DW_OP_stack_value)
+
+; TODO: Find a more-reduced test case for The fix in WebAssemblyRegColoring
+
+; ModuleID = 'debugtest-opt.c'
+source_filename = "debugtest-opt.c"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32"
+
+@globl = hidden local_unnamed_addr global i32* null, align 4, !dbg !0
+
+; Function Attrs: nounwind writeonly
+define hidden void @baz(i32 %arg) local_unnamed_addr #0 !dbg !12 {
+entry:
+  %locl = alloca i32, align 4
+  call void @llvm.dbg.value(metadata i32 %arg, metadata !16, metadata !DIExpression()), !dbg !18
+  %0 = bitcast i32* %locl to i8*, !dbg !19
+  store i32* %locl, i32** @globl, align 4, !dbg !20, !tbaa !21
+  ret void, !dbg !25
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+attributes #0 = { nounwind writeonly "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { argmemonly nounwind willreturn }
+attributes #2 = { nounwind readnone speculatable willreturn }
+attributes #3 = { nounwind }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "globl", scope: !2, file: !3, line: 2, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git ee80f8bef31e0f98c9a0e1d79dc5f1ff51ed9e3a)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, nameTableKind: None)
+!3 = !DIFile(filename: "debugtest-opt.c", directory: "/s/llvm-upstream")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 32)
+!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!8 = !{i32 7, !"Dwarf Version", i32 4}
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{i32 1, !"wchar_size", i32 4}
+!11 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project.git ee80f8bef31e0f98c9a0e1d79dc5f1ff51ed9e3a)"}
+!12 = distinct !DISubprogram(name: "baz", scope: !3, file: !3, line: 14, type: !13, scopeLine: 14, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !15)
+!13 = !DISubroutineType(types: !14)
+!14 = !{null, !7}
+!15 = !{!16, !17}
+!16 = !DILocalVariable(name: "arg", arg: 1, scope: !12, file: !3, line: 14, type: !7)
+!17 = !DILocalVariable(name: "locl", scope: !12, file: !3, line: 15, type: !7)
+!18 = !DILocation(line: 0, scope: !12)
+!19 = !DILocation(line: 15, column: 3, scope: !12)
+!20 = !DILocation(line: 16, column: 9, scope: !12)
+!21 = !{!22, !22, i64 0}
+!22 = !{!"any pointer", !23, i64 0}
+!23 = !{!"omnipotent char", !24, i64 0}
+!24 = !{!"Simple C/C++ TBAA"}
+!25 = !DILocation(line: 17, column: 1, scope: !12)