[llvm] r282886 - [WebAssembly] Make register stackification more conservative

Derek Schuff via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 30 11:02:55 PDT 2016


Author: dschuff
Date: Fri Sep 30 13:02:54 2016
New Revision: 282886

URL: http://llvm.org/viewvc/llvm-project?rev=282886&view=rev
Log:
[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

Modified:
    llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
    llvm/trunk/test/CodeGen/WebAssembly/userstack.ll

Modified: llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp?rev=282886&r1=282885&r2=282886&view=diff
==============================================================================
--- llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp (original)
+++ llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp Fri Sep 30 13:02:54 2016
@@ -274,11 +274,11 @@ static bool HasOneUse(unsigned Reg, Mach
 // 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 MachineIn
       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 MachineIn
 
   // 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 MachineIn
       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::runOnMachin
         // 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);

Modified: llvm/trunk/test/CodeGen/WebAssembly/userstack.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/WebAssembly/userstack.ll?rev=282886&r1=282885&r2=282886&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/WebAssembly/userstack.ll (original)
+++ llvm/trunk/test/CodeGen/WebAssembly/userstack.ll Fri Sep 30 13:02:54 2016
@@ -232,6 +232,42 @@ define void @dynamic_static_alloca(i32 %
  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:




More information about the llvm-commits mailing list