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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits