[PATCH] D60748: Adds an option "malign-pass-aggregate" to make the alignment of the struct and union parameters compatible with the default gcc

[LiuChen via Phabricator via cfe-commits]

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