[flang-commits] [flang] [flang] Fix RecordType sizes, TRANSFER lowering, and BIND(C) ABI on SystemZ/PPC64le (PR #220377)

[via flang-commits]

================
@@ -31,6 +31,61 @@ page](https://llvm.org/releases/).
 
 ## Bug Fixes
 
+- Fixed `fir::getTypeSizeAndAlignment` returning the wrong allocation size for
+  **packed `fir::RecordType`s** (produced by the AIX lowering of `BIND(C)`
+  derived types, or declared directly in textual FIR). Fields in a packed
+  record are placed back-to-back using each component's allocation size
+  (`alignTo(storeSize, ABIalign)`), not its raw store size, and the record's
+  ABI alignment is 1. For example, a packed `{i32, f64}` on x86-64 now
+  correctly reports 12 bytes instead of 16.
+  ([#220377](https://github.com/llvm/llvm-project/pull/220377))
+
+- Fixed `fir::getTypeSizeAndAlignment` omitting **tail padding** from unpacked
+  derived types. The returned size is now rounded up to the record's own ABI
+  alignment, matching the allocation extent used by array element strides, CUDA
+  shared-memory layout, and stack/heap allocation placement. For example,
+  `{i32, i8}` (store size 5 bytes, align 4) now correctly reports 8 bytes
+  instead of 5.
+  ([#220377](https://github.com/llvm/llvm-project/pull/220377))
+
+- Fixed a **`BIND(C)` / `VALUE` argument-passing ABI bug** on SystemZ:
+  derived types whose allocation size fits in a GPR were incorrectly passed
+  indirectly (by reference) instead of as an integer register value, because
+  `getTypeSizeAndAlignment` was returning the unpadded store size rather than
+  the allocation size. For example, `{i32, i8}` (allocation size 8 bytes) is
+  now correctly passed as `i64`, and `{i16, i8}` (4 bytes) as `i32`, matching
+  the C ABI.
+  Fortran programs with `BIND(C)` `VALUE` derived-type arguments of these shapes
+  that interoperate with C were already producing incorrect results; programs
+  compiled entirely in Fortran that relied on the old (incorrect) convention
+  must be recompiled.
+  ([#220377](https://github.com/llvm/llvm-project/pull/220377))
+
+- Fixed a **`BIND(C)` / `VALUE` argument-passing ABI bug** on PPC64le:
+  derived types were classified using the unpadded store size rather than the
+  allocation size, producing the wrong number of GPR slots. The argument was
+  already passed by value; only the slot count was wrong. For example,
+  `{f128, i8}` (allocation size 32 bytes) is now correctly passed as
+  `[4 x i64]` instead of `[3 x i64]`, matching the C ABI.
+  Fortran programs with `BIND(C)` `VALUE` derived-type arguments of these shapes
+  that interoperate with C were already producing incorrect results; programs
+  compiled entirely in Fortran that relied on the old (incorrect) convention
+  must be recompiled.
+  ([#220377](https://github.com/llvm/llvm-project/pull/220377))
+
+- Fixed the `TRANSFER` intrinsic inline path to compare **stored-representation
+  widths** (excluding outer tail padding) rather than allocation sizes when
----------------
jeanPerier wrote:

While the previous inline implementation was already ignoring the padding, I am not sure this is correct. The TRANSFER specification is mentioning "storage size", and it is not defining the other "physical representation" anywhere as far as I can see. To me the padding should be part of the logic so that TRANSFER between arrays and scalar is consistent.

It also seems that other compilers, at least gfortran, is transferring the padding as shown in:

I am not saying the previous inline code was correct since I agree it should do a copy in terms of bytes to avoid having to deal with llvm.store/load sizes.
But I am not sure we should enshrine the previous behavior of not transferring the padding as the correct one. At least that is not my reading of the standard.

Take the following example. Flang is the only compiler to not properly round trip the transfer between `char(8)` and `struct {int a; char b}` (with or without the patch):

```
program transfer_bindc_tail_pad
  use iso_c_binding, only: c_int, c_int8_t, c_sizeof
  implicit none
  type, bind(c) :: t
    integer(c_int)    :: a
    integer(c_int8_t) :: b
  end type
  type(t) :: x
  character(len=:), allocatable :: raw
  integer :: i, n
  n = int(c_sizeof(x))
  print *, 'C_SIZEOF(x) =', n, ' STORAGE_SIZE bits =', storage_size(x)
  allocate(character(len=n) :: raw)
  raw = repeat(achar(255), n)          ! poison the whole C object
  x = transfer(raw, x)
  x%a = int(z'01020304', c_int)        ! LE: 04 03 02 01
  x%b = int(z'AA', c_int8_t)
  raw = transfer(x, raw)
  print '(a)', 'TRANSFER(x, char(C_SIZEOF(x))) bytes:'
  do i = 1, n
    print '(i2,1x,z2.2)', i, iachar(raw(i:i))
  end do
end program
```
See https://godbolt.org/z/rKGdExP7o

I also think that the runtime TRANSFER and the inline implementation should be the same. It should always be correct to use the runtime.



https://github.com/llvm/llvm-project/pull/220377