[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

Wed Jul 19 13:56:12 PDT 2023

hvdijk added a comment.

In D86310#4516184 <https://reviews.llvm.org/D86310#4516184>, @tmgross wrote:

> Is the compatibility note here only meant to address calls between LLVM and GCC, not generated code? Because of course, struct layout and pass-in-memory function calls are incompatible.

There should be no compatibility issue there between GCC and clang in most cases, because clang ensures __int128 is aligned to 16 bytes everywhere, even if the LLVM data layout specifies lower alignment. clang's __int128 and LLVM's i128 play by different rules, currently. This change would make them play by the same rules.

> Rust just uses LLVM's `i128` value directly so it doesn't necessarily need to be called out on its own (think we are in agreement here, just clarifying)

I do think it needs to be called out on its own: Rust makes its

>> [...]
>> QUESTIONS
>>
>> Is the behaviour of LLVM, clang, GCC, and MSVC indeed as I described, or did I make a mistake anywhere?
>
> I believe that MSVC is in general ambiguous about these details on types that it does not support, but I would assume that being consistent with the Linux ABI is preferred and probably what MSVC would choose if they ever do decide on a specification for this type (unless LLVM has contact with Microsoft that may be able to clarify? They make no guarantees against breaking things in any case.)
>
>> [...]
>
> It probably makes sense to have reasoning for choosing the selected behavior and having something specific to test against, so I'll link what I know.
>
> - From AMD4 ABI Draft 0.99.7 (2014) <https://www.uclibc.org/docs/psABI-x86_64.pdf>:
>
>> [paraphrased from Figure 3.1]
>> type - sizeof - alignment - AMD64 architecture
>> long - 8 - 8 - signed eightbyte [I included this in the table for the below reference]
>> `__int128` - 16 - 16 - signed sixteenbyte
>> signed `__int128` - 16 - 16 - signed sixteenbyte
>> `long double` - 16 - 16 - 80-bit extended (IEEE-754)
>> `__float128` - 16 - 16 - 128-bit extended (IEEE-754)
>> [...]
>> The `__int128` type is stored in little-endian order in memory, i.e., the 64
>> low-order bits are stored at a a lower address than the 64 high-order bits
>> [...]
>> Arguments of type `__int128` offer the same operations as INTEGERs,
>> yet they do not fit into one general purpose register but require two registers.
>> For classification purposes `__int128` is treated as if it were implemented
>> as:
>>
>>   typedef struct {
>>       long low, high;
>>   } __int128;
>>
>> with the exception that arguments of type `__int128` that are stored in
>> memory must be aligned on a 16-byte boundary
>
>
>
> - K1OM agrees https://www.intel.com/content/dam/develop/external/us/en/documents/k1om-psabi-1-0.pdf
> - These types don't seem to be mentioned anywhere in i386 1997 https://www.sco.com/developers/devspecs/abi386-4.pdf
> - Also not in MIPS RISC 1996 https://math-atlas.sourceforge.net/devel/assembly/mipsabi32.pdf
> - MIPSpro64 doesn't mention 128-bit integers but does mention 128-bit floats. From page 24 https://math-atlas.sourceforge.net/devel/assembly/mipsabi64.pdf
>
>> Quad-precision floating point parameters (C long double or Fortran REAL*16) are
>> always 16-byte aligned. This requires that they be passed in even-odd floating point
>> register pairs, even if doing so requires skipping a register parameter and/or a
>> 64-bit save area slot. [The 32-bit ABI does not consider long double parameters,
>> since they were not supported.]
>
>
>
> - From PPC64 section 3.1.4 https://math-atlas.sourceforge.net/devel/assembly/PPC-elf64abi-1.7.pdf:
>
>> [paraphrased from table]
>> type - sizeof - alignment
>> `__int128_t` - 16 - quadword
>> `__uint128_t` - 16 - quadword
>> `long double` - 16 - quadword
>
>
>
> - z/Arch: this is the only target that clang seems to align to 8, see [1]. Also from 1.1.2.4 in https://github.com/IBM/s390x-abi/releases/tag/v1.6:
>
>> [paraphrased from table]
>> type - size (bytes) - alignment
>> `__int128` - 16 - 8
>> signed `__int128` - 16 - 8
>> `long double` - 16 - 8
>
> [1]: https://reviews.llvm.org/D130900

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86310