[PATCH] Implement the x86 lowering for inalloca

[Reid Kleckner]

Thanks!

On Fri, Jan 3, 2014 at 4:53 PM, Dan Gohman <sunfish at google.com> wrote:

>
>   > Index: include/llvm/Target/TargetCallingConv.h
>   > ===================================================================
>   > --- include/llvm/Target/TargetCallingConv.h
>   > +++ include/llvm/Target/TargetCallingConv.h
>   > @@ -42,6 +42,8 @@
>   >      static const uint64_t ByValAlignOffs = 7;
>   >      static const uint64_t Split          = 1ULL<<11;
>   >      static const uint64_t SplitOffs      = 11;
>   > +    static const uint64_t InAlloca       = 1ULL<<12; ///< Passed in
> alloca
>   > +    static const uint64_t InAllocaOffs   = 12;
>
>   Change the comment to say "Passed with inalloca" since inalloca means a
> bunch of things beyond just using an alloca.
>

Done.


>   > Index: include/llvm/Target/TargetLowering.h
>   > ===================================================================
>   > --- include/llvm/Target/TargetLowering.h
>   > +++ include/llvm/Target/TargetLowering.h
>   > @@ -24,6 +24,7 @@
>   >  #define LLVM_TARGET_TARGETLOWERING_H
>   >
>   >  #include "llvm/ADT/DenseMap.h"
>   > +#include "llvm/CodeGen/CallingConvLower.h"
>
>   Is this #include needed? It looks like it's for CallLoweringInfo and
> CCState, which could
>   be forward-declared.
>

Looks like it's not needed now.


>   > Index: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>   > ===================================================================
>   > --- lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>   > +++ lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>   [...]
>   > +  // Dynamically allocate space for the stack frame.
>   > +  // TODO: Can we support args aligned to greater than the stack
> alignment?
>   > +  unsigned Align = 0;
>
>   The (unchecked) assumption that default stack alignment is enough seems
> dangerous. How hard would it be to implement this? Is it just a matter of
> computing the greatest alignment from the arguments, or does it require
> additional codegen support?
>

LLVM's byval doesn't support alignments greater than stack alignment, so I
followed suit for inalloca.

Clang clamps its alignments at the stack alignment without emitting a
diagnostic:
http://llvm.org/bugs/show_bug.cgi?id=18140

I *suppose* we could align the DYNAMIC_STACKALLOC node and codegen might
just work, but it seems dicey.  =/


>   [...]
>   > +      if (Args[i].isInAlloca) {
>   > +        Flags.setByVal();  // TODO: Remove when isInAlloca() works.
>
>   Does isInAlloca not work yet?
>
>   > Index: lib/Target/X86/X86ISelLowering.cpp
>   > ===================================================================
>   > --- lib/Target/X86/X86ISelLowering.cpp
>   > +++ lib/Target/X86/X86ISelLowering.cpp
>   > @@ -2501,6 +2501,21 @@
>   >    return Chain;
>   >  }
>   >
>   > +void
> X86TargetLowering::AnalyzeCallArgs(TargetLowering::CallLoweringInfo &CLI,
>   > +                                        CCState &CCInfo) const {
>   > +  SelectionDAG &DAG                     = CLI.DAG;
>   > +  SmallVectorImpl<ISD::OutputArg> &Outs = CLI.Outs;
>   > +  CallingConv::ID CallConv              = CLI.CallConv;
>   > +  bool isVarArg                         = CLI.IsVarArg;
>   > +  MachineFunction &MF = DAG.getMachineFunction();
>
>   DAG, isVarArg, and MF are unused here.
>

Woops.


> http://llvm-reviews.chandlerc.com/D2450
>
> BRANCH
>   inalloca-codegen
>
> ARCANIST PROJECT
>   llvm
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140114/1a51bc30/attachment.html>