<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">Thanks!</div><div class="gmail_quote"><br></div><div class="gmail_quote">On Fri, Jan 3, 2014 at 4:53 PM, Dan Gohman <span dir="ltr"><<a href="mailto:sunfish@google.com" target="_blank">sunfish@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
  > Index: include/llvm/Target/TargetCallingConv.h<br>
  > ===================================================================<br>
  > --- include/llvm/Target/TargetCallingConv.h<br>
  > +++ include/llvm/Target/TargetCallingConv.h<br>
  > @@ -42,6 +42,8 @@<br>
  >      static const uint64_t ByValAlignOffs = 7;<br>
  >      static const uint64_t Split          = 1ULL<<11;<br>
  >      static const uint64_t SplitOffs      = 11;<br>
  > +    static const uint64_t InAlloca       = 1ULL<<12; ///< Passed in alloca<br>
  > +    static const uint64_t InAllocaOffs   = 12;<br>
<br>
  Change the comment to say "Passed with inalloca" since inalloca means a bunch of things beyond just using an alloca.<br></blockquote><div><br></div><div>Done.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

  > Index: include/llvm/Target/TargetLowering.h<br>
  > ===================================================================<br>
  > --- include/llvm/Target/TargetLowering.h<br>
  > +++ include/llvm/Target/TargetLowering.h<br>
  > @@ -24,6 +24,7 @@<br>
  >  #define LLVM_TARGET_TARGETLOWERING_H<br>
  ><br>
  >  #include "llvm/ADT/DenseMap.h"<br>
  > +#include "llvm/CodeGen/CallingConvLower.h"<br>
<br>
  Is this #include needed? It looks like it's for CallLoweringInfo and CCState, which could<br>
  be forward-declared.<br></blockquote><div><br></div><div>Looks like it's not needed now.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

  > Index: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp<br>
  > ===================================================================<br>
  > --- lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp<br>
  > +++ lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp<br>
  [...]<br>
  > +  // Dynamically allocate space for the stack frame.<br>
  > +  // TODO: Can we support args aligned to greater than the stack alignment?<br>
  > +  unsigned Align = 0;<br>
<br>
  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?<br>
</blockquote><div><br></div><div>LLVM's byval doesn't support alignments greater than stack alignment, so I followed suit for inalloca.</div><div><br></div><div>Clang clamps its alignments at the stack alignment without emitting a diagnostic:</div>
<div><a href="http://llvm.org/bugs/show_bug.cgi?id=18140">http://llvm.org/bugs/show_bug.cgi?id=18140</a><br></div><div><br></div><div>I *suppose* we could align the DYNAMIC_STACKALLOC node and codegen might just work, but it seems dicey.  =/</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
  [...]<br>
  > +      if (Args[i].isInAlloca) {<br>
  > +        Flags.setByVal();  // TODO: Remove when isInAlloca() works.<br>
<br>
  Does isInAlloca not work yet?<br>
<br>
  > Index: lib/Target/X86/X86ISelLowering.cpp<br>
  > ===================================================================<br>
  > --- lib/Target/X86/X86ISelLowering.cpp<br>
  > +++ lib/Target/X86/X86ISelLowering.cpp<br>
  > @@ -2501,6 +2501,21 @@<br>
  >    return Chain;<br>
  >  }<br>
  ><br>
  > +void X86TargetLowering::AnalyzeCallArgs(TargetLowering::CallLoweringInfo &CLI,<br>
  > +                                        CCState &CCInfo) const {<br>
  > +  SelectionDAG &DAG                     = CLI.DAG;<br>
  > +  SmallVectorImpl<ISD::OutputArg> &Outs = CLI.Outs;<br>
  > +  CallingConv::ID CallConv              = CLI.CallConv;<br>
  > +  bool isVarArg                         = CLI.IsVarArg;<br>
  > +  MachineFunction &MF = DAG.getMachineFunction();<br>
<br>
  DAG, isVarArg, and MF are unused here.<br></blockquote><div><br></div><div>Woops.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<a href="http://llvm-reviews.chandlerc.com/D2450" target="_blank">http://llvm-reviews.chandlerc.com/D2450</a><br>
<br>
BRANCH<br>
  inalloca-codegen<br>
<br>
ARCANIST PROJECT<br>
  llvm<br>
</blockquote></div><br></div></div>