[WebAssembly] Make register stackification more conservative

Register stackification currently checks VNInfo for changes. Make that
more accurate by testing each intervening instruction for any other defs
to the same virtual register.

Patch by Jacob Gravelle

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@282886 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp b/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
index 047ce4104f75c6cd0071c4950604738e37e0aea8..6dae653af5907c0bcb011ed63b0113249495dea3 100644 (file)
--- a/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
+++ b/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
@@ -274,11 +274,11 @@ static bool HasOneUse(unsigned Reg, MachineInstr *Def,
 // TODO: Compute memory dependencies in a way that uses AliasAnalysis to be
 // more precise.
 static bool IsSafeToMove(const MachineInstr *Def, const MachineInstr *Insert,
-                         AliasAnalysis &AA, const LiveIntervals &LIS,
-                         const MachineRegisterInfo &MRI) {
+                         AliasAnalysis &AA, const MachineRegisterInfo &MRI) {
   assert(Def->getParent() == Insert->getParent());
 
   // Check for register dependencies.
+  SmallVector<unsigned, 4> MutableRegisters;
   for (const MachineOperand &MO : Def->operands()) {
     if (!MO.isReg() || MO.isUndef())
       continue;
@@ -301,21 +301,11 @@ static bool IsSafeToMove(const MachineInstr *Def, const MachineInstr *Insert,
       return false;
     }
 
-    // Ask LiveIntervals whether moving this virtual register use or def to
-    // Insert will change which value numbers are seen.
-    //
-    // If the operand is a use of a register that is also defined in the same
-    // instruction, test that the newly defined value reaches the insert point,
-    // since the operand will be moving along with the def.
-    const LiveInterval &LI = LIS.getInterval(Reg);
-    VNInfo *DefVNI =
-        (MO.isDef() || Def->definesRegister(Reg)) ?
-        LI.getVNInfoAt(LIS.getInstructionIndex(*Def).getRegSlot()) :
-        LI.getVNInfoBefore(LIS.getInstructionIndex(*Def));
-    assert(DefVNI && "Instruction input missing value number");
-    VNInfo *InsVNI = LI.getVNInfoBefore(LIS.getInstructionIndex(*Insert));
-    if (InsVNI && DefVNI != InsVNI)
-      return false;
+    // If one of the operands isn't in SSA form, it has different values at
+    // different times, and we need to make sure we don't move our use across
+    // a different def.
+    if (!MO.isDef() && !MRI.hasOneDef(Reg))
+      MutableRegisters.push_back(Reg);
   }
 
   bool Read = false, Write = false, Effects = false, StackPointer = false;
@@ -323,7 +313,8 @@ static bool IsSafeToMove(const MachineInstr *Def, const MachineInstr *Insert,
 
   // If the instruction does not access memory and has no side effects, it has
   // no additional dependencies.
-  if (!Read && !Write && !Effects && !StackPointer)
+  bool HasMutableRegisters = !MutableRegisters.empty();
+  if (!Read && !Write && !Effects && !StackPointer && !HasMutableRegisters)
     return true;
 
   // Scan through the intervening instructions between Def and Insert.
@@ -343,6 +334,11 @@ static bool IsSafeToMove(const MachineInstr *Def, const MachineInstr *Insert,
       return false;
     if (StackPointer && InterveningStackPointer)
       return false;
+
+    for (unsigned Reg : MutableRegisters)
+      for (const MachineOperand &MO : I->operands())
+        if (MO.isReg() && MO.isDef() && MO.getReg() == Reg)
+          return false;
   }
 
   return true;
@@ -781,7 +777,7 @@ bool WebAssemblyRegStackify::runOnMachineFunction(MachineFunction &MF) {
         // 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, LIS, MRI) &&
+        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);

diff --git a/test/CodeGen/WebAssembly/userstack.ll b/test/CodeGen/WebAssembly/userstack.ll
index 234e647f196298b6f6c05681320a2c431657f7d7..44c5da537166d50a96af496c52dde2118952cb43 100644 (file)
--- a/test/CodeGen/WebAssembly/userstack.ll
+++ b/test/CodeGen/WebAssembly/userstack.ll
@@ -232,6 +232,42 @@ define void @dynamic_static_alloca(i32 %alloc) noredzone {
  ret void
 }
 
+declare i8* @llvm.stacksave()
+declare void @llvm.stackrestore(i8*)
+
+; CHECK-LABEL: llvm_stack_builtins:
+define void @llvm_stack_builtins(i32 %alloc) noredzone {
+ ; CHECK: i32.load $push[[L11:.+]]=, __stack_pointer($pop{{.+}})
+ ; CHECK-NEXT: tee_local $push[[L10:.+]]=, ${{.+}}=, $pop[[L11]]
+ ; CHECK-NEXT: copy_local $[[STACK:.+]]=, $pop[[L10]]
+ %stack = call i8* @llvm.stacksave()
+
+ ; Ensure we don't reassign the stacksave local
+ ; CHECK-NOT: $[[STACK]]=
+ %dynamic = alloca i32, i32 %alloc
+
+ ; CHECK: i32.store $drop=, __stack_pointer($pop{{.+}}), $[[STACK]]
+ call void @llvm.stackrestore(i8* %stack)
+
+ ret void
+}
+
+; Not actually using the alloca'd variables exposed an issue with register
+; stackification, where copying the stack pointer into the frame pointer was
+; moved after the stack pointer was updated for the dynamic alloca.
+; CHECK-LABEL: dynamic_alloca_nouse:
+define void @dynamic_alloca_nouse(i32 %alloc) noredzone {
+ ; CHECK: i32.load $push[[L11:.+]]=, __stack_pointer($pop{{.+}})
+ ; CHECK-NEXT: tee_local $push[[L10:.+]]=, ${{.+}}=, $pop[[L11]]
+ ; CHECK-NEXT: copy_local $[[FP:.+]]=, $pop[[L10]]
+ %dynamic = alloca i32, i32 %alloc
+
+ ; CHECK-NOT: $[[FP]]=,
+
+ ; CHECK: i32.store $drop=, __stack_pointer($pop{{.+}}), $[[FP]]
+ ret void
+}
+
 ; The use of the alloca in a phi causes a CopyToReg DAG node to be generated,
 ; which has to have special handling because CopyToReg can't have a FI operand
 ; CHECK-LABEL: copytoreg_fi: