[PATCH] D130900: [SystemZ] Make 128 bit integers be aligned to 8 bytes.

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

In D130900#3693772 <https://reviews.llvm.org/D130900#3693772>, @jonpa wrote:

> @uweigand I also added tests for the alignment of a vector depending on the presence of the vector facility. I am not sure if I have run into something weird here, but I left the test and output this way for now so you can see what I mean: If I use -mattr=-vector the alignment becomes 16 for the vector, but not if this is specified in the function attributes only (see @f2).

Yes, this is because of the way `computeDataLayout` in SystemZTargetMachine.cpp currently gives different resuts based on the presence of the vector feature - but this routine is only used once per compilation unit, not once per function.   This is actually a long-standing problem, the data layout really ought to depend only on the triple, but right now it doesn't.  (If you recall, that's also why we wanted to emit a warning if files with different vector feature setting are linked together ...)

However, given what we just found about i128 (that it had always been set to 64-bit alignment in the data layout - it was just the front end that aligned to 128-bit anyway!), maybe the proper fix to the vector ABI issue would be to also set vector alignment to 64 bits in the data layout / the back end unconditionally, and only make the 128-bit / 64-bit distinction in the clang front-end!     But that is a certainly a separate discussion.

Given this, maybe it's best to remove the vector type from this patch after all, and work on that separately.


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

https://reviews.llvm.org/D130900