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