[WebAssemly] Don't move calls or stores past intervening loads

[dschuff]

Summary

The register stackifier currently checks for intervening stores (and
loads that may alias them) but doesn't account for the fact that the
instruction being moved may affect intervening loads.

[sunfish]

Good catch. The isSafeToMove API is pretty confusing; this is what comes of trying to be clever with it. I think we'll eventually want to revisit this code and do something other than isSafeToMove, but this change looks ok for now.

llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp

VNInfo *DefVNI =
         MO.isDef() ? 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;
   }
 
+  SawStore = Def->isCall() || Def->mayStore();
   // Check for memory dependencies and side effects.
   for (--I; I != D; --I)
     SawSideEffects |= !I->isSafeToMove(&AA, SawStore);
   return !(SawStore && Def->mayLoad() && !Def->isInvariantLoad(&AA)) &&
          !(SawSideEffects && !Def->isSafeToMove(&AA, SawStore));
 }
 
 /// Test whether OneUse, a use of Reg, dominates all of Reg's other uses.

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

bb17:                                             ; preds = %bb13, %bb8
   br i1 %tmp20, label %bb8, label %bb21
 
 bb21:                                             ; preds = %bb17, %bb5
   %tmp22 = phi double [ %tmp, %bb5 ], [ %tmp9, %bb17 ]
   %tmp23 = fadd double %tmp6, 1.000000e+00
   br label %bb5
 }
 
+; Don't move calls past loads
+; CHECK-LABEL: no_stackify_call_past_load:
+; CHECK: i32.call $0=, red
+; CHECK: i32.const $push0=, 0
+; CHECK: i32.load $1=, count($pop0)
+@count = hidden global i32 0, align 4
+define i32 @no_stackify_call_past_load() {
+  %a = call i32 @red()
+  %b = load i32, i32* @count, align 4
+  call i32 @callee(i32 %a)
+  ret i32 %b
+  ; use of a
+}
+
+; Don't move stores past loads if there may be aliasing
+; CHECK-LABEL: no_stackify_store_past_load
+; CHECK: i32.store {{.*}}, 0($1), $0
+; CHECK: i32.load {{.*}}, 0($2)
+; CHECK: i32.call {{.*}}, callee@FUNCTION, $0
+define i32 @no_stackify_store_past_load(i32 %a, i32* %p1, i32* %p2) {
+  store i32 %a, i32* %p1
+  %b = load i32, i32* %p2, align 4
+  call i32 @callee(i32 %a)
+  ret i32 %b
+}
+
+; Can still stackify past invariant loads.
+; CHECK-LABEL: store_past_invar_load
+; CHECK: i32.store $push{{.*}}, 0($1), $0
+; CHECK: i32.call {{.*}}, callee@FUNCTION, $pop
+; CHECK: i32.load $push{{.*}}, 0($2)
+; CHECK: return $pop
+define i32 @store_past_invar_load(i32 %a, i32* %p1, i32* dereferenceable(4) %p2) {
+  store i32 %a, i32* %p1
+  %b = load i32, i32* %p2, !invariant.load !0
+  call i32 @callee(i32 %a)
+  ret i32 %b
+}
+
 !0 = !{}