[PATCH] D12337: [Codegen] Ensure stack is properly aligned for call argument initialization

[Jeroen Ketema via llvm-commits]

jketema added inline comments.

================
Comment at: include/llvm/CodeGen/CallingConvLower.h:274-279
@@ -272,4 +273,8 @@
 
-  unsigned getNextStackOffset() const { return StackOffset; }
+  /// getNextStackOffset - Return the stack offset needed to be able to store
+  /// all stack slots according to their alignment requirements.
+  unsigned getNextStackOffset() const {
+    return RoundUpToAlignment(StackOffset, MaxStackArgAlign);
+  }
 
   /// isAllocated - Return true if the specified register (or an alias) is
----------------
jketema wrote:
> rnk wrote:
> > Yes, but here's the 32-bit test case I'm imagining:
> >   void f(__m128 v, const char *f, ...) {
> >     va_list ap;
> >     va_start(ap, f);
> >     int d = va_arg(ap, int);
> >   }
> > Do we have padding between 'f' and the variadic int parameter? I think the answer should be know, because if they were not variadic, they would not have padding between them.
> > 
> > 
> Sorry, I missed this before I bumped. Somehow the system didn't sent me a notification email.
> 
> I don't see why this is a problem, my patch only changes the alignment of the whole frame, not the padding between the individual members; the computation for that stays unchanged. Hence, there will only be additional padding after the very last of the variadic parameters supplied.
Reading back all the comments, I think I see what you're getting at: What you're saying is that `getNextStackOffset` is also used to compute the offset of the variadic arguments with respect to the beginning of the stack frame?

If that's the case, then the patch would indeed be wrong, and I think we would then need both the old variant and the new one I'm proposing and adapt every use accordingly. Or do you see a better solution?


http://reviews.llvm.org/D12337