[all-commits] [llvm/llvm-project] a26ba3: [flang] Fix TRANSFER into derived type with tail p...
Daniel Chen via All-commits
all-commits at lists.llvm.org
Wed Sep 23 12:21:57 PDT 2026
Branch: refs/heads/main
Home: https://github.com/llvm/llvm-project
Commit: a26ba32319c157f5b8d6f59528ec001db9ff4055
https://github.com/llvm/llvm-project/commit/a26ba32319c157f5b8d6f59528ec001db9ff4055
Author: Daniel Chen <cdchen at ca.ibm.com>
Date: 2026-09-23 (Wed, 23 Sep 2026)
Changed paths:
M flang/lib/Optimizer/Builder/FIRBuilder.cpp
M flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp
M flang/test/Fir/CUDA/predefined-variables.mlir
M flang/test/Fir/copy-codegen.fir
M flang/test/HLFIR/assign-codegen-derived.fir
M flang/test/Lower/CUDA/cuda-devptr.cuf
M flang/test/Lower/Intrinsics/transfer.f90
M flang/test/Lower/OpenACC/acc-firstprivate-derived-user-assign.f90
M flang/test/Lower/OpenACC/acc-firstprivate-derived.f90
Log Message:
-----------
[flang] Fix TRANSFER into derived type with tail padding (#223814)
When assigning a derived type value whose allocated size exceeds its
declared
field size (e.g. `SEQUENCE` type `{i32, i8}` — 5 bytes of fields, 8
bytes
allocated due to ABI tail padding), the assignment path in
`genRecordAssignment` took a "simple type" fast path that called
`genComponentByComponentAssignment`. That function iterated over
declared
fields and copied them one by one via `fir.coordinate_of`/`fir.load`/
`fir.store`, silently skipping the tail-padding bytes and leaving them
**unwritten** in the destination.
This is a compatibility choice rather than a correctness fix: F2023
10.2.1.3
p15 defines derived-type intrinsic assignment component by component,
and p16
allows any means with the same effect, so padding is outside the defined
effect
on either side. gfortran preserves the full allocated storage (including
padding
bytes) on assignment; this patch aligns flang's behaviour with gfortran
for two
cases:
- **`TRANSFER` into a padded SEQUENCE type**: `x = transfer(raw, x)` now
copies
all transferred bytes — including those that fall in the padding region
— into
`x`. A round-trip `raw = transfer(x, raw)` then recovers the original
value.
- **Structure constructor**: `x = t(1, 2_c_int8_t)` now copies the full
temporary, so the padding bytes written into `x` are the temporary's
padding
bytes rather than being left unwritten.
Fix: replace `genComponentByComponentAssignment` with `fir::CopyOp`
(emitting
`fir.copy`). `fir.copy` defers the size computation to codegen via
`genTypeStrideInBytes` and the LLVM data layout, which includes tail
padding.
The dead `genComponentByComponentAssignment` function (≈80 lines) is
removed.
### no_overlap / memmove
`no_overlap` is only sound when the operands are *known* to be
non-overlapping. The `!recTy.isSequence()` fast-path condition is an
*assumption* rather than a proven fact: `flang/docs/Aliasing.md`
documents a
configuration where a Cray pointee aliases a variable with the `TARGET`
attribute, producing overlapping operands even for a non-SEQUENCE type.
A separate `noOverlap` flag is `true` only when non-overlap is *proven*
—
either the LHS is a compiler temporary, or `fir::AliasAnalysis`
explicitly
returns `NoAlias`. When `noOverlap` is `false`, `fir.copy` lowers to
`memmove`, which is correct for all overlap configurations. The
`disjoint`
gate (which selects `fir.copy` over the `_FortranAAssign` runtime path)
is
unchanged.
### Volatile operands
The ref type for each `fir.copy` operand is computed using
`builder.getRefType(recTy, fir::isa_volatile_type(addr.getType()))`, so
a
`fir.convert` is only emitted when the element type actually differs
(e.g. a
module record type assigned to a structurally identical local SEQUENCE
type)
and volatility is always preserved. This ensures whole-record assignment
to or
from a `volatile` derived-type variable passes `ConvertOp::verify` under
`--strict-fir-volatile-verifier`.
### CUFPredefinedVarToGPU
`processDeclareOp` previously cast every use of a predefined-variable
`fir.declare` to `fir.coordinate_of` without a null check, crashing when
the
use was anything else. A whole-record assignment such as `idx =
threadIdx`
lowers to `fir.copy %threadidx to %idx`, producing a non-coordinate use
that
triggered the crash.
`fir.copy` uses of a predefined-variable declare are now expanded into
three
per-field NVVM register reads + stores, so the destination receives
actual GPU
register values rather than a copy from the global backing variable.
Each
field store uses a reference type that inherits the destination's
volatility,
so a `volatile` destination (e.g. `type(dim3), volatile :: idx`)
produces
volatile stores and passes `--strict-fir-volatile-verifier`. The declare
and
the global are erased once all uses are handled; `fir.call` and other
unrecognised uses keep the declare alive.
### Known follow-ups
`fir.copy` does not yet implement `FirAliasTagOpInterface` (recorded as
a TODO
in `FIROps.td`) and its lowering does not forward alignment from the
operand
types. Every simple derived-type assignment now routed through
`fir.copy` —
including the OpenACC/OpenMP recipes and the CUDA paths changed by this
patch —
loses its per-dummy TBAA tags and emits `align 1` instead of the natural
field
alignment. Both losses are conservative and the code remains correct,
but they
may affect optimisation quality at `-O2` and above. Implementing
`FirAliasTagOpInterface` and propagating alignment are left as follow-up
work.
### Example
```fortran
program transfer_bindc_tail_pad
use iso_c_binding, only: c_int, c_int8_t, c_sizeof
implicit none
type :: t
sequence
integer(c_int) :: a
integer(c_int8_t) :: b
end type
type(t) :: x
integer, parameter :: n = int(STORAGE_SIZE(x)/8)
character(len=n) :: raw, rawx
equivalence (x, rawx)
print *, ' STORAGE_SIZE({i32, 8}) bits =', storage_size(x)
rawx = repeat(achar(0), n)
raw = repeat(achar(255), n)
print *, "raw before round trip"
x = transfer(raw, x)
print *, "intermediate round trip value"
call printbytes(rawx)
print *, "raw after round trip"
raw = transfer(x, raw)
call printbytes(raw)
contains
subroutine printbytes(c)
character(*) :: c
integer :: i
do i = 1, n
print '(i2,1x,z2.2)', i, iachar(c(i:i))
end do
end subroutine
end program
```
Before this patch (padding bytes left unwritten; the zeros come from
`rawx = repeat(achar(0), n)` — with a different initialiser the values
would differ):
```
> a.out
STORAGE_SIZE({i32, 8}) bits = 64
raw before round trip
intermediate round trip value
1 FF
2 FF
3 FF
4 FF
5 FF
6 00
7 00
8 00
raw after round trip
1 FF
2 FF
3 FF
4 FF
5 FF
6 00
7 00
8 00
```
After this patch, consistent with gfortran (source bytes fully
preserved):
```
> a.out
STORAGE_SIZE({i32, 8}) bits = 64
raw before round trip
intermediate round trip value
1 FF
2 FF
3 FF
4 FF
5 FF
6 FF
7 FF
8 FF
raw after round trip
1 FF
2 FF
3 FF
4 FF
5 FF
6 FF
7 FF
8 FF
```
Assisted-by: IBM Bob
To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications
More information about the All-commits
mailing list