hubert.reinterpretcast 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 | \
----------------
Xiangling_L wrote:
> 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`?
>
Yes, we can scope the patch that way somewhat; however, some cases of "[using
the `__alignof__` value] where we should" that is missing is within the
determination of the base and field offsets. We should keep those within the
scope of this patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79719/new/
https://reviews.llvm.org/D79719
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
