rjmccall added inline comments.

================
Comment at: include/clang/Basic/TargetCXXABI.h:118
@@ -115,1 +117,3 @@
+    /// in LLVM 3.2.
+    PS4
   };
----------------
probinson wrote:
> rjmccall wrote:
> > I'm not sure why you added a new C++ ABI kind here.  The bug fix you're 
> > opting out of is not at all specific to C++, and there are more 
> > straightforward ways to check the target than checking the C++ ABI kind.
> > 
> > I mean, I have no doubt that eventually there will be some significant C++ 
> > ABI bug fix that you don't want to pick up, so I'm not opposed to adding a 
> > new C++ ABI kind.  It just seems inappropriate to do that in this patch.
> The alignment attribute is affecting layout, and layout would seem to be an 
> ABI thing.
The CXXABI types dictate C++-specific ABI details, e.g. the layout of v-tables. 
 Generic C-level ABI details like type sizes and alignments and struct layout 
quirks are provided by TargetInfo.  In particular, TargetInfo already have 3 or 
4 target-specific methods controlling various details of bit-field alignment; 
adding another for this will be very natural.


http://reviews.llvm.org/D16788



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to