[llvm] r366765 - [Statepoints] Fix a bug in statepoint lowering for functions w/no-realign-stack

[Philip Reames via llvm-commits]

Thank you for catching this and fixing it.  Danger of only building R+A.

Oddly, I didn't receive a single build bot notification for this failure.

Philip

On 7/22/19 8:14 PM, Richard Trieu wrote:
>
>
> On Mon, Jul 22, 2019 at 4:32 PM Philip Reames via llvm-commits 
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>
>     Author: reames
>     Date: Mon Jul 22 16:33:18 2019
>     New Revision: 366765
>
>     URL: http://llvm.org/viewvc/llvm-project?rev=366765&view=rev
>     Log:
>     [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.  :)
>
>
>     Modified:
>     llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>     llvm/trunk/test/CodeGen/X86/statepoint-no-realign-stack.ll
>
>     Modified: llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>     URL:
>     http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp?rev=366765&r1=366764&r2=366765&view=diff
>     ==============================================================================
>     --- llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>     (original)
>     +++ llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp Mon
>     Jul 22 16:33:18 2019
>     @@ -389,10 +389,17 @@ spillIncomingStatepointValue(SDValue Inc
>                 "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));
>
> MFI here refers to a variable above that is in a debug only section, 
> which fails to build in non-debug builds.  In r366773 that section is 
> no longer debug only to fix the build.
>
>          Chain = Builder.DAG.getStore(Chain, Builder.getCurSDLoc(),
>     Incoming, Loc,
>     -                                 PtrInfo);
>     +                                 StoreMMO);
>
>          MMO = getMachineMemOperand(MF, *cast<FrameIndexSDNode>(Loc));
>
>
>     Modified: llvm/trunk/test/CodeGen/X86/statepoint-no-realign-stack.ll
>     URL:
>     http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/statepoint-no-realign-stack.ll?rev=366765&r1=366764&r2=366765&view=diff
>     ==============================================================================
>     --- llvm/trunk/test/CodeGen/X86/statepoint-no-realign-stack.ll
>     (original)
>     +++ llvm/trunk/test/CodeGen/X86/statepoint-no-realign-stack.ll Mon
>     Jul 22 16:33:18 2019
>     @@ -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:
>
>
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>     https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190723/dc912c01/attachment.html>