From 2f5543aa725c1b5af17a65b229e2573260d7cbb6 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Mon, 22 Jul 2019 23:33:18 +0000 Subject: [PATCH] [Statepoints] Fix a bug in statepoint lowering for functions w/no-realign-stack We were silently using the ABI alignment for all of the stores generated for deopt and gc values. We'd gotten the alignment of the stack slot itself properly reduced (via MachineFrameInfo's clamping), but having the MMO on the store incorrect was enough for us to generate an aligned store to a unaligned location. The simplest fix would have been to just pass the alignment to the helper function, but once we do that, the helper function doesn't really help. So, inline it and directly call the MMO version of DAG.getStore with a properly constructed MMO. Note that there's a separate performance possibility here. Even if we *can* realign stacks, we probably don't *want to* if all of the stores are in slowpaths. But that's a later patch, if at all. :) llvm-svn: 366765 --- llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp | 9 ++++++++- llvm/test/CodeGen/X86/statepoint-no-realign-stack.ll | 3 +-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp index 395e9a8..12716de 100644 --- a/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp @@ -389,10 +389,17 @@ spillIncomingStatepointValue(SDValue Incoming, SDValue Chain, "Bad spill: stack slot does not match!"); #endif + // Note: Using the alignment of the spill slot (rather than the abi or + // preferred alignment) is required for correctness when dealing with spill + // slots with preferred alignments larger than frame alignment.. auto &MF = Builder.DAG.getMachineFunction(); auto PtrInfo = MachinePointerInfo::getFixedStack(MF, Index); + auto *StoreMMO = + MF.getMachineMemOperand(PtrInfo, MachineMemOperand::MOStore, + MFI.getObjectSize(Index), + MFI.getObjectAlignment(Index)); Chain = Builder.DAG.getStore(Chain, Builder.getCurSDLoc(), Incoming, Loc, - PtrInfo); + StoreMMO); MMO = getMachineMemOperand(MF, *cast(Loc)); diff --git a/llvm/test/CodeGen/X86/statepoint-no-realign-stack.ll b/llvm/test/CodeGen/X86/statepoint-no-realign-stack.ll index 943d877..3c2411b 100644 --- a/llvm/test/CodeGen/X86/statepoint-no-realign-stack.ll +++ b/llvm/test/CodeGen/X86/statepoint-no-realign-stack.ll @@ -35,14 +35,13 @@ define void @can_realign(<8 x i32>* %p) { ret void } -; TODO: currently shows incorrect codegen, FIXME define void @no_realign(<8 x i32>* %p) "no-realign-stack" { ; CHECK-LABEL: no_realign: ; CHECK: # %bb.0: ; CHECK-NEXT: subq $40, %rsp ; CHECK-NEXT: .cfi_def_cfa_offset 48 ; CHECK-NEXT: vmovaps (%rdi), %ymm0 -; CHECK-NEXT: vmovaps %ymm0, (%rsp) +; CHECK-NEXT: vmovups %ymm0, (%rsp) ; CHECK-NEXT: vzeroupper ; CHECK-NEXT: callq foo ; CHECK-NEXT: .Ltmp1: -- 2.7.4