[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 5 inline comments as done.
Xiangling_L added inline comments.


================
Comment at: clang/test/Layout/aix-double-struct-member.cpp:1
+// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \
+// RUN:     -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \
----------------
hubert.reinterpretcast wrote:
> Xiangling_L wrote:
> > hubert.reinterpretcast wrote:
> > > I am concerned that none of the tests actually create an instance of the classes under test and check the alignment (or related adjustments) in the IR. That is, we set up the preferred alignment value but don't check that we use it where we should.
> > > 
> > > As it is, it seems array new/delete has problems:
> > > ```
> > > #include <assert.h>
> > > extern "C" void *calloc(decltype(sizeof 0), decltype(sizeof 0));
> > > extern "C" void free(void *);
> > > extern "C" int printf(const char *, ...);
> > > 
> > > extern void *allocated_ptr;
> > > extern decltype(sizeof 0) allocated_size;
> > > struct B {
> > >   double d;
> > >   ~B() {}
> > >   static void *operator new[](decltype(sizeof 0) sz);
> > >   static void operator delete[](void *p, decltype(sizeof 0) sz);
> > > };
> > > B *allocBp();
> > > 
> > > #ifdef ALLOCBP
> > > void *allocated_ptr;
> > > decltype(sizeof 0) allocated_size;
> > > void *B::operator new[](decltype(sizeof 0) sz) {
> > >   void *alloc = calloc(1u, allocated_size = sz);
> > >   printf("%p: %s\n", alloc, __PRETTY_FUNCTION__);
> > >   printf("%zu\n", sz);
> > >   return allocated_ptr = alloc;
> > > }
> > > void B::operator delete[](void *p, decltype(sizeof 0) sz) {
> > >   printf("%p: %s\n", p, __PRETTY_FUNCTION__);
> > >   printf("%zu\n", sz);
> > >   assert(sz == allocated_size);
> > >   assert(p == allocated_ptr);
> > >   free(p);
> > > }
> > > B *allocBp() { return new B[2]; }
> > > #endif
> > > 
> > > #ifdef MAIN
> > > int main(void) { delete[] allocBp(); }
> > > #endif
> > > ```
> > > 
> > > The `xlclang++` invocation from XL C/C++ generates padding before the 32-bit `new[]` cookie. I'm not seeing that padding with this patch.
> > Thank. I will create more practical testcases as you mentioned in your concern. And regarding to `padding before the 32-bit new[] cookie` issue, I am wondering is that part of `power` alignment rule or what rules do we follow to generate this kind of padding?
> The padding has to do with the alignment. The allocation function returns 8-byte aligned memory. The 32-bit cookie takes 4 of the first 8 bytes. The type's preferred alignment is 8, so there are 4 bytes of padding.
Regarding with checking the alignment where we use them, AFAIK the problematic cases include not only the `cookie padding` issue you mentioned here, but also the alignment of argument type, return type etc.

So I am wondering does it make sense to have them handled in a separate patch since this is already a big one? We can use this patch to implement the correct value of `__alignof` and `alignof` and use a second patch to handle the places where `we use them where we should`?



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79719/new/

https://reviews.llvm.org/D79719