LiuChen3 marked 3 inline comments as done.
LiuChen3 added inline comments.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:1559
+ if (const auto *AT = QT->getAsArrayTypeUnsafe())
+ TempAlignment = getContext().getTypeAlign(AT->getElementType()) /
8;
+ else // recursively to get each type's alignment
----------------
jyknight wrote:
> Also needs to call getTypeStackAlignInBytes?
I think this is enough.
```
struct __attribute__((aligned(16))) X6 {
int x;
struct X1 x1[5];
// alignedint64 y;
};
void test(int a, struct X6 x6)
{
printf(%u\n", __alignof__(x6));
}
```
This will output 64.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:1567
+ if (MaxAlignment >= 16)
+ return std::max(MaxAlignment, Align);
+ else // return 4 when all the type alignments less than 16 bytes
----------------
jyknight wrote:
> I think this is wrong and that it should only return Align. The computation
> of the alignment of the elements is only to see if their alignment is >= 16.
>
> If the alignment of the elements' types is >= 16, but the alignment of the
> structure is less than the alignment of one of its elements (e.g. due to
> `__attribute__ packed`), we should return the alignment of the structure.
I write a test, I do n’t know if it matches your meaning.
```
struct __attribute__((aligned(4))) X6 {
int x;
alignedint64 y;
};
void test(int a, struct X6 x6)
{
printf("%u\n", __alignof__(x6));
}
```
The output of gcc is 64.
If we use packed attribute:
```
struct __attribute__((packed)) X6 {
int x;
alignedint64 y;
};
```
Both gcc and clang with this patch output "1". And I found that the packed
struct is not processed by this function.
So I think it should return the MaxAlignment .
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:1570
+ return 4;
+ } else if (Align < 16)
+ return MinABIStackAlignInBytes;
----------------
jyknight wrote:
> If I understood GCC's algorithm correctly, I think this needs to come first?
You mean it should be ?
```
if (MaxAlignment < 16)
retrun 4
else
return std::max(MaxAlignment, Align);
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60748/new/
https://reviews.llvm.org/D60748
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
