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

[Richard Trieu via llvm-commits]

On Mon, Jul 22, 2019 at 4:32 PM Philip Reames via llvm-commits <
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
> 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/20190722/2894e8ca/attachment.html>