From: Dan Gohman Date: Sat, 5 Dec 2015 00:51:40 +0000 (+0000) Subject: [WebAssembly] Fix scheduling dependencies in register-stackified code X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=ca526959c1fc8d8172b780d916cdeb3f03a33836 [WebAssembly] Fix scheduling dependencies in register-stackified code Add physical register defs to instructions used from stackified instructions to prevent them from being scheduled into the middle of a stack sequence. This is a conservative measure which may be loosened in the future. Differential Revision: http://reviews.llvm.org/D15252 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@254811 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp b/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp index 7abc20a8387..ac016a7b9b0 100644 --- a/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp +++ b/lib/Target/WebAssembly/WebAssemblyRegStackify.cpp @@ -61,15 +61,41 @@ FunctionPass *llvm::createWebAssemblyRegStackify() { } // Decorate the given instruction with implicit operands that enforce the -// expression stack ordering constraints. -static void ImposeStackOrdering(MachineInstr *MI) { - // Read and write the opaque EXPR_STACK register. - MI->addOperand(MachineOperand::CreateReg(WebAssembly::EXPR_STACK, - /*isDef=*/true, - /*isImp=*/true)); +// expression stack ordering constraints needed for an instruction which is +// consumed by an instruction using the expression stack. +static void ImposeStackInputOrdering(MachineInstr *MI) { + // Write the opaque EXPR_STACK register. + if (!MI->definesRegister(WebAssembly::EXPR_STACK)) + MI->addOperand(MachineOperand::CreateReg(WebAssembly::EXPR_STACK, + /*isDef=*/true, + /*isImp=*/true)); +} + +// Decorate the given instruction with implicit operands that enforce the +// expression stack ordering constraints for an instruction which is on +// the expression stack. +static void ImposeStackOrdering(MachineInstr *MI, MachineRegisterInfo &MRI) { + ImposeStackInputOrdering(MI); + + // Also read the opaque EXPR_STACK register. MI->addOperand(MachineOperand::CreateReg(WebAssembly::EXPR_STACK, /*isDef=*/false, /*isImp=*/true)); + + // Also, mark any inputs to this instruction as being consumed by an + // instruction on the expression stack. + // TODO: Find a lighter way to describe the appropriate constraints. + for (MachineOperand &MO : MI->uses()) { + if (!MO.isReg()) + continue; + unsigned Reg = MO.getReg(); + if (!TargetRegisterInfo::isVirtualRegister(Reg)) + continue; + MachineInstr *Def = MRI.getVRegDef(Reg); + if (Def->getOpcode() == TargetOpcode::PHI) + continue; + ImposeStackInputOrdering(Def); + } } // Test whether it's safe to move Def to just before Insert. Note that this @@ -126,8 +152,15 @@ bool WebAssemblyRegStackify::runOnMachineFunction(MachineFunction &MF) { continue; unsigned Reg = Op.getReg(); - if (!TargetRegisterInfo::isVirtualRegister(Reg)) + if (TargetRegisterInfo::isPhysicalRegister(Reg)) { + // An instruction with a physical register. Conservatively mark it as + // an expression stack input so that it isn't reordered with anything + // in an expression stack which might use it (physical registers + // aren't in SSA form so it's not trivial to determine this). + // TODO: Be less conservative. + ImposeStackInputOrdering(Insert); continue; + } // Only consider registers with a single definition. // TODO: Eventually we may relax this, to stackify phi transfers. @@ -178,11 +211,11 @@ bool WebAssemblyRegStackify::runOnMachineFunction(MachineFunction &MF) { MBB.insert(MachineBasicBlock::instr_iterator(Insert), Def->removeFromParent()); MFI.stackifyVReg(Reg); - ImposeStackOrdering(Def); + ImposeStackOrdering(Def, MRI); Insert = Def; } if (AnyStackified) - ImposeStackOrdering(&MI); + ImposeStackOrdering(&MI, MRI); } } diff --git a/test/CodeGen/WebAssembly/load-store-i1.ll b/test/CodeGen/WebAssembly/load-store-i1.ll index 1acdfc0dbde..37b51472947 100644 --- a/test/CodeGen/WebAssembly/load-store-i1.ll +++ b/test/CodeGen/WebAssembly/load-store-i1.ll @@ -15,8 +15,8 @@ define i32 @load_u_i1_i32(i1* %p) { } ; CHECK-LABEL: load_s_i1_i32: -; CHECK: i32.load8_u $push[[NUM0:[0-9]+]]=, 0($0){{$}} -; CHECK-NEXT: i32.const $[[NUM1:[0-9]+]]=, 31{{$}} +; CHECK: i32.const $[[NUM1:[0-9]+]]=, 31{{$}} +; CHECK-NEXT: i32.load8_u $push[[NUM0:[0-9]+]]=, 0($0){{$}} ; CHECK-NEXT: shl $push[[NUM2:[0-9]+]]=, $pop[[NUM0]], $[[NUM1]]{{$}} ; CHECK-NEXT: shr_s $push[[NUM3:[0-9]+]]=, $pop[[NUM2]], $[[NUM1]]{{$}} ; CHECK-NEXT: return $pop[[NUM3]]{{$}} @@ -36,8 +36,8 @@ define i64 @load_u_i1_i64(i1* %p) { } ; CHECK-LABEL: load_s_i1_i64: -; CHECK: i64.load8_u $push[[NUM0:[0-9]+]]=, 0($0){{$}} -; CHECK-NEXT: i64.const $[[NUM1:[0-9]+]]=, 63{{$}} +; CHECK: i64.const $[[NUM1:[0-9]+]]=, 63{{$}} +; CHECK-NEXT: i64.load8_u $push[[NUM0:[0-9]+]]=, 0($0){{$}} ; CHECK-NEXT: shl $push[[NUM2:[0-9]+]]=, $pop[[NUM0]], $[[NUM1]]{{$}} ; CHECK-NEXT: shr_s $push[[NUM3:[0-9]+]]=, $pop[[NUM2]], $[[NUM1]]{{$}} ; CHECK-NEXT: return $pop[[NUM3]]{{$}} diff --git a/test/CodeGen/WebAssembly/reg-stackify.ll b/test/CodeGen/WebAssembly/reg-stackify.ll index f3000aab70a..180d70e2e4a 100644 --- a/test/CodeGen/WebAssembly/reg-stackify.ll +++ b/test/CodeGen/WebAssembly/reg-stackify.ll @@ -44,4 +44,32 @@ define i32 @yes1(i32* %q) { ret i32 %t } +; Don't schedule stack uses into the stack. To reduce register pressure, the +; scheduler might be tempted to move the definition of $2 down. However, this +; would risk getting incorrect liveness if the instructions are later +; rearranged to make the stack contiguous. + +; CHECK-LABEL: stack_uses: +; CHECK-NEXT: .param i32{{$}} +; CHECK-NEXT: .result i32{{$}} +; CHECK-NEXT: local i32, i32{{$}} +; CHECK-NEXT: i32.const $1=, 1{{$}} +; CHECK-NEXT: i32.const $2=, 0{{$}} +; CHECK-NEXT: i32.and $push0=, $0, $1{{$}} +; CHECK-NEXT: i32.eq $push1=, $pop0, $2{{$}} +; CHECK-NEXT: block BB4_2{{$}} +; CHECK-NEXT: br_if $pop1, BB4_2{{$}} +; CHECK-NEXT: return $2{{$}} +; CHECK-NEXT: BB4_2:{{$}} +; CHECK-NEXT: return $1{{$}} +define i32 @stack_uses(i32 %x) { +entry: + %c = trunc i32 %x to i1 + br i1 %c, label %true, label %false +true: + ret i32 0 +false: + ret i32 1 +} + !0 = !{}