[PATCH] D19756: Expand aggregate arguments more often on 32-bit Windows

[Hans Wennborg via cfe-commits]

hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

This is awesome! lgtm

I only have nits and questions.

Want to reference PR27522 in the patch description?

Also in the description:

> I also expanded the workaround handle C++ records with constructors on


is missing a "to".


================
Comment at: lib/CodeGen/TargetInfo.cpp:1134
@@ +1133,3 @@
+
+/// canExpandIndirectArgument - Test whether an argument type which is to be
+/// passed indirectly (on the stack) would have the equivalent layout if it was
----------------
maybe drop the function name from the comment while here

================
Comment at: lib/CodeGen/TargetInfo.cpp:1151
@@ +1150,3 @@
+    } else {
+      // Don't do this for dynamic classes.
+      if (CXXRD->isDynamicClass())
----------------
Why not for dynamic classes or non-empty bases? Is it just that they hold more data than the immediate fields, or is there some other reason?

================
Comment at: lib/CodeGen/TargetInfo.cpp:1177
@@ +1176,3 @@
+
+  // We can do this if there was no alignment padding.
+  return Size == getContext().getTypeSize(Ty);
----------------
Could there be a situation where the struct is packed, but there would be padding when we expand it?

I suppose it won't happen in practice since we only allow 32/64-bit members..

================
Comment at: lib/CodeGen/TargetInfo.cpp:1397
@@ -1397,1 +1396,3 @@
                                              bool &NeedsPadding) const {
+  // On Windows, aggregates other than HFAs are never passed in registers, and
+  // they do not consume register slots. HFAs have already been dealt with at
----------------
Silly question: what's an HFA? (Also for the patch description)

================
Comment at: test/CodeGen/windows-struct-abi.c:13
@@ -12,3 +12,3 @@
 
-// CHECK: define void @receive_f1(%struct.f1* byval align 4 %a0)
+// CHECK: define void @receive_f1(float %a0.0)
 
----------------
we're sure this float won't end up in a register right?


http://reviews.llvm.org/D19756