[flang-commits] [flang] [flang] Fix TRANSFER into derived type with tail padding zeroing pad bytes (PR #223814)
Daniel Chen via flang-commits
flang-commits at lists.llvm.org
Thu Sep 17 00:03:27 PDT 2026
https://github.com/DanielCChen updated https://github.com/llvm/llvm-project/pull/223814
>From dad1d47da0eaf7d88e534092beea780f4d4bdf92 Mon Sep 17 00:00:00 2001
From: Daniel Chen <cdchen at ca.ibm.com>
Date: Tue, 15 Sep 2026 16:16:41 -0400
Subject: [PATCH 1/2] [flang] Fix TRANSFER into derived type with tail padding
zeroing pad bytes
---
flang/lib/Optimizer/Builder/FIRBuilder.cpp | 107 +++---------------
flang/test/HLFIR/assign-codegen-derived.fir | 20 +++-
flang/test/Lower/CUDA/cuda-devptr.cuf | 8 +-
flang/test/Lower/Intrinsics/transfer.f90 | 21 ++++
.../acc-firstprivate-derived-user-assign.f90 | 19 +---
.../OpenACC/acc-firstprivate-derived.f90 | 19 +---
6 files changed, 59 insertions(+), 135 deletions(-)
diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
index ee33c172494e9..7c15366569149 100644
--- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp
+++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -1396,86 +1396,6 @@ void fir::factory::genScalarAssignment(
}
}
-static void genComponentByComponentAssignment(fir::FirOpBuilder &builder,
- mlir::Location loc,
- const fir::ExtendedValue &lhs,
- const fir::ExtendedValue &rhs,
- bool isTemporaryLHS) {
- auto lbaseType = fir::unwrapPassByRefType(fir::getBase(lhs).getType());
- auto lhsType = mlir::dyn_cast<fir::RecordType>(lbaseType);
- assert(lhsType && "lhs must be a scalar record type");
- auto rbaseType = fir::unwrapPassByRefType(fir::getBase(rhs).getType());
- auto rhsType = mlir::dyn_cast<fir::RecordType>(rbaseType);
- assert(rhsType && "rhs must be a scalar record type");
- auto fieldIndexType = fir::FieldType::get(lhsType.getContext());
- for (auto [lhsPair, rhsPair] :
- llvm::zip(lhsType.getTypeList(), rhsType.getTypeList())) {
- auto &[lFieldName, lFieldTy] = lhsPair;
- auto &[rFieldName, rFieldTy] = rhsPair;
- assert(!fir::hasDynamicSize(lFieldTy) && !fir::hasDynamicSize(rFieldTy));
- mlir::Value rField =
- fir::FieldIndexOp::create(builder, loc, fieldIndexType, rFieldName,
- rhsType, fir::getTypeParams(rhs));
- auto rFieldRefType = builder.getRefType(rFieldTy);
- mlir::Value fromCoor = fir::CoordinateOp::create(
- builder, loc, rFieldRefType, fir::getBase(rhs), rField);
- mlir::Value field =
- fir::FieldIndexOp::create(builder, loc, fieldIndexType, lFieldName,
- lhsType, fir::getTypeParams(lhs));
- auto fieldRefType = builder.getRefType(lFieldTy);
- mlir::Value toCoor = fir::CoordinateOp::create(builder, loc, fieldRefType,
- fir::getBase(lhs), field);
- std::optional<fir::DoLoopOp> outerLoop;
- if (auto sequenceType = mlir::dyn_cast<fir::SequenceType>(lFieldTy)) {
- // Create loops to assign array components elements by elements.
- // Note that, since these are components, they either do not overlap,
- // or are the same and exactly overlap. They also have compile time
- // constant shapes.
- mlir::Type idxTy = builder.getIndexType();
- llvm::SmallVector<mlir::Value> indices;
- mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
- mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
- for (auto extent : llvm::reverse(sequenceType.getShape())) {
- // TODO: add zero size test !
- mlir::Value ub = builder.createIntegerConstant(loc, idxTy, extent - 1);
- auto loop = fir::DoLoopOp::create(builder, loc, zero, ub, one);
- if (!outerLoop)
- outerLoop = loop;
- indices.push_back(loop.getInductionVar());
- builder.setInsertionPointToStart(loop.getBody());
- }
- // Set indices in column-major order.
- std::reverse(indices.begin(), indices.end());
- auto elementRefType = builder.getRefType(sequenceType.getEleTy());
- toCoor = fir::CoordinateOp::create(builder, loc, elementRefType, toCoor,
- indices);
- fromCoor = fir::CoordinateOp::create(builder, loc, elementRefType,
- fromCoor, indices);
- }
- if (auto fieldEleTy = fir::unwrapSequenceType(lFieldTy);
- mlir::isa<fir::BaseBoxType>(fieldEleTy)) {
- assert(mlir::isa<fir::PointerType>(
- mlir::cast<fir::BaseBoxType>(fieldEleTy).getEleTy()) &&
- "allocatable members require deep copy");
- auto fromPointerValue = fir::LoadOp::create(builder, loc, fromCoor);
- auto castTo = builder.createConvert(loc, fieldEleTy, fromPointerValue);
- fir::StoreOp::create(builder, loc, castTo, toCoor);
- } else {
- auto from =
- fir::factory::componentToExtendedValue(builder, loc, fromCoor);
- auto to = fir::factory::componentToExtendedValue(builder, loc, toCoor);
- // If LHS finalization is needed it is expected to be done
- // for the parent record, so that component-by-component
- // assignments may avoid finalization calls.
- fir::factory::genScalarAssignment(builder, loc, to, from,
- /*needFinalization=*/false,
- isTemporaryLHS);
- }
- if (outerLoop)
- builder.setInsertionPointAfter(*outerLoop);
- }
-}
-
/// Can the assignment of this record type be implement with a simple memory
/// copy (it requires no deep copy or user defined assignment of components )?
static bool recordTypeCanBeMemCopied(fir::RecordType recordType) {
@@ -1552,15 +1472,24 @@ void fir::factory::genRecordAssignment(fir::FirOpBuilder &builder,
return;
}
- // Otherwise, the derived type has compile time constant size and for which
- // the component by component assignment can be replaced by a memory copy.
- // Since we do not know the size of the derived type in lowering, do a
- // component by component assignment. Note that a single fir.load/fir.store
- // could be used on "small" record types, but as the type size grows, this
- // leads to issues in LLVM (long compile times, long IR files, and even
- // asserts at some point). Since there is no good size boundary, just always
- // use component by component assignment here.
- genComponentByComponentAssignment(builder, loc, lhs, rhs, isTemporaryLHS);
+ // Otherwise, the derived type has compile time constant size, no
+ // allocatable components, and no user-defined assignment. The size of the
+ // type is not known at this point in lowering, but fir.copy defers the size
+ // computation to codegen where the LLVM data layout is available. That allows
+ // it to copy the full allocated storage including any ABI tail-padding bytes,
+ // which a field-by-field copy would silently skip. Preserving those bytes
+ // matters for SEQUENCE types whose storage is reinterpreted via EQUIVALENCE
+ // or TRANSFER.
+ mlir::Value fromAddr = fir::getBase(rhs);
+ mlir::Value toAddr = fir::getBase(lhs);
+ // Ensure we have raw ref<RecordType> pointers for fir.copy.
+ auto refTy = builder.getRefType(recTy);
+ if (fromAddr.getType() != refTy)
+ fromAddr = builder.createConvert(loc, refTy, fromAddr);
+ if (toAddr.getType() != refTy)
+ toAddr = builder.createConvert(loc, refTy, toAddr);
+ // disjoint == true at this point (guaranteed by the condition above).
+ fir::CopyOp::create(builder, loc, fromAddr, toAddr, /*noOverlap=*/true);
}
mlir::Value fir::factory::createZeroValue(fir::FirOpBuilder &builder,
diff --git a/flang/test/HLFIR/assign-codegen-derived.fir b/flang/test/HLFIR/assign-codegen-derived.fir
index 9bba0d31a6ea6..f6dbb010140fa 100644
--- a/flang/test/HLFIR/assign-codegen-derived.fir
+++ b/flang/test/HLFIR/assign-codegen-derived.fir
@@ -12,10 +12,8 @@ func.func @test_simple(%a: !fir.ref<!t_simple>, %b: !fir.ref<!t_simple>) {
}
// CHECK-LABEL: func.func @test_simple(
// CHECK-NOT: Destroy
-// CHECK: %[[VAL_1:.*]] = fir.coordinate_of %{{.*}}, i : (!fir.ref<!fir.type<simple{i:i32}>>) -> !fir.ref<i32>
-// CHECK: %[[VAL_3:.*]] = fir.coordinate_of %{{.*}}, i : (!fir.ref<!fir.type<simple{i:i32}>>) -> !fir.ref<i32>
-// CHECK: %[[VAL_4:.*]] = fir.load %[[VAL_1]] : !fir.ref<i32>
-// CHECK: fir.store %[[VAL_4]] to %[[VAL_3]] : !fir.ref<i32>
+// CHECK-NOT: fir.coordinate_of
+// CHECK: fir.copy %{{.*}} to %{{.*}} no_overlap : !fir.ref<!fir.type<simple{i:i32}>>, !fir.ref<!fir.type<simple{i:i32}>>
!t_with_final = !fir.type<with_final{i:i32}>
@@ -27,3 +25,17 @@ func.func @test_with_final(%a: !fir.ref<!t_with_final>, %b: !fir.ref<!t_with_fin
}
// CHECK-LABEL: func.func @test_with_final(
// CHECK: fir.call @_FortranAAssign
+
+// Derived type with two fields of different sizes: the LLVM struct will have
+// tail padding. The assignment must use fir.copy (full allocated size) rather
+// than field-by-field copy so that tail-padding bytes are preserved.
+!t_tail_pad = !fir.type<tail_pad,sequence{a:i32,b:i8}>
+fir.type_info @tail_pad noinit nodestroy nofinal : !t_tail_pad
+
+func.func @test_tail_pad(%a: !fir.ref<!t_tail_pad>, %b: !fir.ref<!t_tail_pad>) {
+ hlfir.assign %b to %a : !fir.ref<!t_tail_pad>, !fir.ref<!t_tail_pad>
+ return
+}
+// CHECK-LABEL: func.func @test_tail_pad(
+// CHECK-NOT: fir.coordinate_of
+// CHECK: fir.copy %{{.*}} to %{{.*}} no_overlap : !fir.ref<!fir.type<tail_pad,sequence{a:i32,b:i8}>>, !fir.ref<!fir.type<tail_pad,sequence{a:i32,b:i8}>>
diff --git a/flang/test/Lower/CUDA/cuda-devptr.cuf b/flang/test/Lower/CUDA/cuda-devptr.cuf
index 924bfa55c7e5b..dd21d08a6e54f 100644
--- a/flang/test/Lower/CUDA/cuda-devptr.cuf
+++ b/flang/test/Lower/CUDA/cuda-devptr.cuf
@@ -66,12 +66,8 @@ end subroutine
! CHECK-LABEL: func.func @_QPassign_c_devptr
! CHECK: %[[P:.*]] = fir.declare %arg0 dummy_scope %{{.*}} {data_attr = #cuf.cuda<device>, uniq_name = "_QFassign_c_devptrEp"}
! CHECK: %[[C_DEVLOC_RES:.*]] = fir.declare %15 {uniq_name = ".tmp.intrinsic_result"} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_devptr{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}>>) -> !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_devptr{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}>>
-! CHECK: %[[RES_CPTR_COORD:.*]] = fir.coordinate_of %[[C_DEVLOC_RES]], cptr : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_devptr{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}>>) -> !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>
-! CHECK: %[[P_CPTR_COORD:.*]] = fir.coordinate_of %[[P]], cptr : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_devptr{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}>>) -> !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>
-! CHECK: %[[RES_ADDR_COORD:.*]] = fir.coordinate_of %[[RES_CPTR_COORD]], __address : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>) -> !fir.ref<i64>
-! CHECK: %[[P_ADDR_COORD:.*]] = fir.coordinate_of %[[P_CPTR_COORD]], __address : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>>) -> !fir.ref<i64>
-! CHECK: %[[ADDR:.*]] = fir.load %[[RES_ADDR_COORD]] : !fir.ref<i64>
-! CHECK: fir.store %[[ADDR]] to %[[P_ADDR_COORD]] : !fir.ref<i64>
+! CHECK-NOT: fir.coordinate_of
+! CHECK: fir.copy %[[C_DEVLOC_RES]] to %[[P]] no_overlap : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_devptr{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}>>, !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_c_devptr{cptr:!fir.type<_QM__fortran_builtinsT__builtin_c_ptr{__address:i64}>}>>
attributes(global) subroutine assign_nested_c_devptr(p, a)
use cudafct
diff --git a/flang/test/Lower/Intrinsics/transfer.f90 b/flang/test/Lower/Intrinsics/transfer.f90
index 7afdfd28c2ae1..a460ed5dfcb4e 100644
--- a/flang/test/Lower/Intrinsics/transfer.f90
+++ b/flang/test/Lower/Intrinsics/transfer.f90
@@ -151,3 +151,24 @@ subroutine trans_test_alloc_source(store, src)
real, allocatable :: src
store = transfer(src, store)
end subroutine
+
+ ! TRANSFER into a SEQUENCE derived type with tail padding: the assignment back
+ ! to the derived-type variable must use fir.copy (full storage size including
+ ! tail padding) rather than a field-by-field copy that silently drops padding.
+ subroutine trans_test_seq_tail_pad(raw, x)
+ ! CHECK-LABEL: func @_QPtrans_test_seq_tail_pad(
+ ! CHECK: fir.call @_FortranATransfer(
+ ! CHECK: hlfir.assign {{.*}} to %[[xDecl:.*]]#0
+ ! CHECK-NOT: hlfir.assign
+ ! CHECK: return
+ ! CHECK: }
+ use iso_c_binding, only: c_int, c_int8_t
+ type :: t
+ sequence
+ integer(c_int) :: a
+ integer(c_int8_t) :: b
+ end type
+ character(len=8), intent(inout) :: raw
+ type(t), intent(out) :: x
+ x = transfer(raw, x)
+ end subroutine
diff --git a/flang/test/Lower/OpenACC/acc-firstprivate-derived-user-assign.f90 b/flang/test/Lower/OpenACC/acc-firstprivate-derived-user-assign.f90
index edf7db534baa9..c1fc2e060b294 100644
--- a/flang/test/Lower/OpenACC/acc-firstprivate-derived-user-assign.f90
+++ b/flang/test/Lower/OpenACC/acc-firstprivate-derived-user-assign.f90
@@ -81,23 +81,6 @@ subroutine test()
! FIR-CHECK: acc.yield %[[VAL_1]] : !fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>
! FIR-CHECK: } copy {
! FIR-CHECK: ^bb0(%[[VAL_0:.*]]: !fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>, %[[VAL_1:.*]]: !fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>):
-! FIR-CHECK: %[[VAL_2:.*]] = fir.field_index x, !fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_3:.*]] = fir.coordinate_of %[[VAL_0]], x : (!fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_4:.*]] = fir.field_index x, !fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_5:.*]] = fir.coordinate_of %[[VAL_1]], x : (!fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_6:.*]] = fir.load %[[VAL_3]] : !fir.ref<f32>
-! FIR-CHECK: fir.store %[[VAL_6]] to %[[VAL_5]] : !fir.ref<f32>
-! FIR-CHECK: %[[VAL_7:.*]] = fir.field_index y, !fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_8:.*]] = fir.coordinate_of %[[VAL_0]], y : (!fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_9:.*]] = fir.field_index y, !fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_10:.*]] = fir.coordinate_of %[[VAL_1]], y : (!fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_11:.*]] = fir.load %[[VAL_8]] : !fir.ref<f32>
-! FIR-CHECK: fir.store %[[VAL_11]] to %[[VAL_10]] : !fir.ref<f32>
-! FIR-CHECK: %[[VAL_12:.*]] = fir.field_index z, !fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_13:.*]] = fir.coordinate_of %[[VAL_0]], z : (!fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_14:.*]] = fir.field_index z, !fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_15:.*]] = fir.coordinate_of %[[VAL_1]], z : (!fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_16:.*]] = fir.load %[[VAL_13]] : !fir.ref<f32>
-! FIR-CHECK: fir.store %[[VAL_16]] to %[[VAL_15]] : !fir.ref<f32>
+! FIR-CHECK: fir.copy %[[VAL_0]] to %[[VAL_1]] no_overlap : !fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>, !fir.ref<!fir.type<_QMm_firstprivate_derived_user_defTpoint{x:f32,y:f32,z:f32}>>
! FIR-CHECK: acc.terminator
! FIR-CHECK: }
diff --git a/flang/test/Lower/OpenACC/acc-firstprivate-derived.f90 b/flang/test/Lower/OpenACC/acc-firstprivate-derived.f90
index 8aa8b3156f3b1..0861483ec61d7 100644
--- a/flang/test/Lower/OpenACC/acc-firstprivate-derived.f90
+++ b/flang/test/Lower/OpenACC/acc-firstprivate-derived.f90
@@ -67,23 +67,6 @@ subroutine test()
! FIR-CHECK: acc.yield %[[VAL_1]] : !fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>
! FIR-CHECK: } copy {
! FIR-CHECK: ^bb0(%[[VAL_0:.*]]: !fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>, %[[VAL_1:.*]]: !fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>):
-! FIR-CHECK: %[[VAL_2:.*]] = fir.field_index x, !fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_3:.*]] = fir.coordinate_of %[[VAL_0]], x : (!fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_4:.*]] = fir.field_index x, !fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_5:.*]] = fir.coordinate_of %[[VAL_1]], x : (!fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_6:.*]] = fir.load %[[VAL_3]] : !fir.ref<f32>
-! FIR-CHECK: fir.store %[[VAL_6]] to %[[VAL_5]] : !fir.ref<f32>
-! FIR-CHECK: %[[VAL_7:.*]] = fir.field_index y, !fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_8:.*]] = fir.coordinate_of %[[VAL_0]], y : (!fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_9:.*]] = fir.field_index y, !fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_10:.*]] = fir.coordinate_of %[[VAL_1]], y : (!fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_11:.*]] = fir.load %[[VAL_8]] : !fir.ref<f32>
-! FIR-CHECK: fir.store %[[VAL_11]] to %[[VAL_10]] : !fir.ref<f32>
-! FIR-CHECK: %[[VAL_12:.*]] = fir.field_index z, !fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_13:.*]] = fir.coordinate_of %[[VAL_0]], z : (!fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_14:.*]] = fir.field_index z, !fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>
-! FIR-CHECK: %[[VAL_15:.*]] = fir.coordinate_of %[[VAL_1]], z : (!fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>) -> !fir.ref<f32>
-! FIR-CHECK: %[[VAL_16:.*]] = fir.load %[[VAL_13]] : !fir.ref<f32>
-! FIR-CHECK: fir.store %[[VAL_16]] to %[[VAL_15]] : !fir.ref<f32>
+! FIR-CHECK: fir.copy %[[VAL_0]] to %[[VAL_1]] no_overlap : !fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>, !fir.ref<!fir.type<_QMm_firstprivate_derivedTpoint{x:f32,y:f32,z:f32}>>
! FIR-CHECK: acc.terminator
! FIR-CHECK: }
>From 73e95b760da04b8a0c79cdfcf546340a39873f20 Mon Sep 17 00:00:00 2001
From: Daniel Chen <cdchen at ca.ibm.com>
Date: Thu, 17 Sep 2026 03:02:55 -0400
Subject: [PATCH 2/2] [flang] Fix volatile, no_overlap, and CUF crash in
derived-type assignment
Address review comments on the fir.copy derived-type assignment patch:
- Volatile operands: compute a per-operand ref type that preserves
volatility so that fir.convert does not strip the volatile qualifier
when the element type differs. Fixes lowering under
--strict-fir-volatile-verifier for whole-record assignment to or from
a volatile derived-type variable.
- no_overlap: introduce a separate noOverlap flag that is true only when
non-overlap is *proven* -- LHS is a compiler temporary, or alias
analysis returns NoAlias. The previous code set no_overlap for all
non-SEQUENCE types, which emits memcpy and has undefined behaviour
when a Cray pointee aliases a TARGET variable. When noOverlap is
false, fir.copy lowers to memmove.
- CUFPredefinedVarToGPU: skip non-fir.coordinate_of uses of a
predefined-variable fir.declare instead of null-dereferencing them.
A whole-record assignment (idx = threadIdx) now lowers to fir.copy
and would crash the pass. The declare is kept alive when such uses
remain.
- Tests: add volatile assignment cases to assign-codegen-derived.fir,
an 8-byte tail-padded SEQUENCE case to copy-codegen.fir, a
bbc -emit-fir RUN line to transfer.f90 checking fir.copy on the
sequence{a:i32,b:i8} type, and a fir.copy use case to
predefined-variables.mlir.
---
flang/lib/Optimizer/Builder/FIRBuilder.cpp | 36 ++++++++++++++----
.../Transforms/CUDA/CUFPredefinedVarToGPU.cpp | 11 +++++-
flang/test/Fir/CUDA/predefined-variables.mlir | 30 +++++++++++++++
flang/test/Fir/copy-codegen.fir | 16 ++++++++
flang/test/HLFIR/assign-codegen-derived.fir | 37 +++++++++++++++++++
flang/test/Lower/Intrinsics/transfer.f90 | 7 ++++
6 files changed, 129 insertions(+), 8 deletions(-)
diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
index 7c15366569149..5793926504112 100644
--- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp
+++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -1454,6 +1454,18 @@ void fir::factory::genRecordAssignment(fir::FirOpBuilder &builder,
bool disjoint = isTemporaryLHS || !recTy.isSequence() ||
(aa.alias(fir::getBase(lhs), fir::getBase(rhs)) ==
mlir::AliasResult::NoAlias);
+ // noOverlap is only safe when the operands are *known* to be
+ // non-overlapping: either the LHS is a compiler temporary (can't alias
+ // anything the user wrote), or alias analysis explicitly confirmed
+ // NoAlias. The !recTy.isSequence() branch of disjoint is an assumption
+ // -- non-SEQUENCE types are expected to be disjoint, but a Cray pointee
+ // aliasing a TARGET variable is a documented counter-example (see
+ // flang/docs/Aliasing.md). Using no_overlap on an assumed-disjoint pair
+ // emits memcpy, which has undefined behaviour for overlapping operands.
+ // Without no_overlap, fir.copy lowers to memmove, which is always safe.
+ bool noOverlap =
+ isTemporaryLHS || (aa.alias(fir::getBase(lhs), fir::getBase(rhs)) ==
+ mlir::AliasResult::NoAlias);
if ((needFinalization && mayHaveFinalizer(recTy, builder)) ||
hasBoxOperands || !recordTypeCanBeMemCopied(recTy) || !disjoint) {
auto to = fir::getBase(builder.createBox(loc, lhs));
@@ -1483,13 +1495,23 @@ void fir::factory::genRecordAssignment(fir::FirOpBuilder &builder,
mlir::Value fromAddr = fir::getBase(rhs);
mlir::Value toAddr = fir::getBase(lhs);
// Ensure we have raw ref<RecordType> pointers for fir.copy.
- auto refTy = builder.getRefType(recTy);
- if (fromAddr.getType() != refTy)
- fromAddr = builder.createConvert(loc, refTy, fromAddr);
- if (toAddr.getType() != refTy)
- toAddr = builder.createConvert(loc, refTy, toAddr);
- // disjoint == true at this point (guaranteed by the condition above).
- fir::CopyOp::create(builder, loc, fromAddr, toAddr, /*noOverlap=*/true);
+ // Use a per-operand target type that preserves the operand's volatility.
+ // If either operand is !fir.ref<T, volatile>, the target type is also
+ // !fir.ref<T, volatile>, so any emitted fir.convert does not strip the
+ // volatile qualifier and passes ConvertOp::verify under
+ // --strict-fir-volatile-verifier. A convert is only emitted when the
+ // element type differs (e.g. a module record type assigned to a
+ // structurally identical local SEQUENCE type); when the types already
+ // match no convert is emitted at all.
+ auto fromRefTy =
+ builder.getRefType(recTy, fir::isa_volatile_type(fromAddr.getType()));
+ if (fromAddr.getType() != fromRefTy)
+ fromAddr = builder.createConvert(loc, fromRefTy, fromAddr);
+ auto toRefTy =
+ builder.getRefType(recTy, fir::isa_volatile_type(toAddr.getType()));
+ if (toAddr.getType() != toRefTy)
+ toAddr = builder.createConvert(loc, toRefTy, toAddr);
+ fir::CopyOp::create(builder, loc, fromAddr, toAddr, noOverlap);
}
mlir::Value fir::factory::createZeroValue(fir::FirOpBuilder &builder,
diff --git a/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp b/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp
index 4f669461d92d8..8c49fc8e9ce7b 100644
--- a/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp
+++ b/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp
@@ -74,9 +74,17 @@ processDeclareOp(mlir::OpBuilder &builder, fir::DeclareOp declareOp,
llvm::SmallVectorImpl<mlir::Operation *> &opsToDelete,
llvm::SmallPtrSetImpl<mlir::Operation *> &memrefDefiningOps) {
if (declareOp.getUniqName().str().compare(builtinVar) == 0) {
+ bool allUsesAreCoords = true;
for (mlir::OpOperand &use : declareOp.getResult().getUses()) {
fir::CoordinateOp coordOp =
mlir::dyn_cast<fir::CoordinateOp>(use.getOwner());
+ if (!coordOp) {
+ // Non-coordinate uses (e.g. fir.copy, fir.call) cannot be rewritten
+ // to GPU register reads here. Leave them in place and keep the
+ // declare alive so those uses remain valid.
+ allUsesAreCoords = false;
+ continue;
+ }
processCoordinateOp<OpTyX>(builder, coordOp, field_x, incrementByOne,
opsToDelete);
processCoordinateOp<OpTyY>(builder, coordOp, field_y, incrementByOne,
@@ -85,7 +93,8 @@ processDeclareOp(mlir::OpBuilder &builder, fir::DeclareOp declareOp,
opsToDelete);
opsToDelete.push_back(coordOp);
}
- opsToDelete.push_back(declareOp.getOperation());
+ if (allUsesAreCoords)
+ opsToDelete.push_back(declareOp.getOperation());
// The backing fir.address_of may be shared by several declares (e.g. after
// CSE coalesces them when a device routine is inlined into a kernel).
// Collect it de-duplicated and erase it only once all declares are gone.
diff --git a/flang/test/Fir/CUDA/predefined-variables.mlir b/flang/test/Fir/CUDA/predefined-variables.mlir
index 3b63128a260d0..ce4af29f4be73 100644
--- a/flang/test/Fir/CUDA/predefined-variables.mlir
+++ b/flang/test/Fir/CUDA/predefined-variables.mlir
@@ -491,3 +491,33 @@ func.func @_QPsub4(%arg0: !fir.ref<i32> {fir.bindc_name = "i", cuf.data_attr = #
// LOC: arith.addi {{.*}} : i32 loc("sub4.cuf":3:3)
// LOC: nvvm.read.ptx.sreg.ctaid.y : i32 loc("sub4.cuf":4:3)
// LOC: arith.addi {{.*}} : i32 loc("sub4.cuf":4:3)
+
+// -----
+
+// A fir.copy use of a predefined-variable declare (e.g. from a whole-record
+// assignment like `idx = threadIdx`) must not crash the pass. The declare
+// and the copy are not rewritten; coordinate-based uses in the same function
+// are still lowered to GPU register reads.
+func.func @_QPcopy_threadidx(%arg0: !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>> {fir.bindc_name = "idx"}) attributes {cuf.proc_attr = #cuf.cuda_proc<global>} {
+ %0 = fir.address_of(@_QM__fortran_builtinsE__builtin_threadidx) : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>>
+ %1 = fir.declare %0 {uniq_name = "_QM__fortran_builtinsE__builtin_threadidx"} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>>) -> !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>>
+ %2 = fir.declare %arg0 {uniq_name = "_QFsub1Eidx"} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>>) -> !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>>
+ // Whole-record assignment: idx = threadIdx
+ fir.copy %1 to %2 no_overlap : !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>>, !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>>
+ // Component read: i = threadIdx%x (must still be rewritten)
+ %3 = fir.coordinate_of %1, x : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>>) -> !fir.ref<i32>
+ %4 = fir.load %3 : !fir.ref<i32>
+ %5 = fir.alloca i32
+ fir.store %4 to %5 : !fir.ref<i32>
+ return
+}
+
+// The declare must survive (fir.copy still uses it).
+// CHECK-LABEL: func.func @_QPcopy_threadidx
+// CHECK: %[[DECL:.*]] = fir.declare %{{.*}} {uniq_name = "_QM__fortran_builtinsE__builtin_threadidx"}
+// The whole-record copy is left intact.
+// CHECK: fir.copy %[[DECL]] to %{{.*}} no_overlap
+// The coordinate use is still lowered to a register read.
+// CHECK: %[[TID:.*]] = nvvm.read.ptx.sreg.tid.x : i32
+// CHECK: %[[ADD:.*]] = arith.addi %[[TID]], %c1{{.*}} : i32
+// CHECK: fir.store %[[ADD]] to %{{.*}} : !fir.ref<i32>
diff --git a/flang/test/Fir/copy-codegen.fir b/flang/test/Fir/copy-codegen.fir
index 7b0620ca2d312..d805bf8ea80ad 100644
--- a/flang/test/Fir/copy-codegen.fir
+++ b/flang/test/Fir/copy-codegen.fir
@@ -2,6 +2,7 @@
// RUN: fir-opt --fir-to-llvm-ir %s -o - | FileCheck %s
!t=!fir.type<sometype{i:!fir.array<9xi32>}>
+!t_tail_pad = !fir.type<tail_pad,sequence{a:i32,b:i8}>
module attributes {llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"} {
@@ -28,4 +29,19 @@ func.func @test_copy_2(%arg0: !fir.ref<!t>, %arg1: !fir.ref<!t>) {
// CHECK: "llvm.intr.memmove"(%[[VAL_1]], %[[VAL_0]], %[[VAL_2]]) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i64) -> ()
// CHECK: llvm.return
// CHECK: }
+
+// SEQUENCE derived type with tail padding: i32 + i8 = 5 bytes of data,
+// aligned to 4 bytes => 8-byte allocated size. fir.copy must use the
+// full allocated size so that the 3 padding bytes are included.
+func.func @test_copy_tail_pad(%arg0: !fir.ref<!t_tail_pad>, %arg1: !fir.ref<!t_tail_pad>) {
+ fir.copy %arg0 to %arg1 no_overlap : !fir.ref<!t_tail_pad>, !fir.ref<!t_tail_pad>
+ return
+}
+// CHECK-LABEL: llvm.func @test_copy_tail_pad(
+// CHECK-SAME: %[[VAL_0:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: !llvm.ptr,
+// CHECK-SAME: %[[VAL_1:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: !llvm.ptr) {
+// CHECK: %[[VAL_2:.*]] = llvm.mlir.constant(8 : i64) : i64
+// CHECK: "llvm.intr.memcpy"(%[[VAL_1]], %[[VAL_0]], %[[VAL_2]]) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i64) -> ()
+// CHECK: llvm.return
+// CHECK: }
}
diff --git a/flang/test/HLFIR/assign-codegen-derived.fir b/flang/test/HLFIR/assign-codegen-derived.fir
index f6dbb010140fa..64533a423f448 100644
--- a/flang/test/HLFIR/assign-codegen-derived.fir
+++ b/flang/test/HLFIR/assign-codegen-derived.fir
@@ -39,3 +39,40 @@ func.func @test_tail_pad(%a: !fir.ref<!t_tail_pad>, %b: !fir.ref<!t_tail_pad>) {
// CHECK-LABEL: func.func @test_tail_pad(
// CHECK-NOT: fir.coordinate_of
// CHECK: fir.copy %{{.*}} to %{{.*}} no_overlap : !fir.ref<!fir.type<tail_pad,sequence{a:i32,b:i8}>>, !fir.ref<!fir.type<tail_pad,sequence{a:i32,b:i8}>>
+
+
+// Volatile derived-type assignment: the operand types must not have their
+// volatility stripped by a fir.convert. Under --strict-fir-volatile-verifier
+// fir.convert rejects a conversion that drops the volatile qualifier.
+
+// RUN: fir-opt --strict-fir-volatile-verifier --convert-hlfir-to-fir %s | FileCheck %s --check-prefix=CHECK-VOL
+
+// Volatile LHS, non-volatile RHS.
+!t_vol = !fir.type<vol_t{i:i32}>
+fir.type_info @vol_t noinit nodestroy nofinal : !t_vol
+
+func.func @test_volatile_lhs(%a: !fir.ref<!t_vol, volatile>, %b: !fir.ref<!t_vol>) {
+ hlfir.assign %b to %a : !fir.ref<!t_vol>, !fir.ref<!t_vol, volatile>
+ return
+}
+// CHECK-VOL-LABEL: func.func @test_volatile_lhs(
+// CHECK-VOL-NOT: fir.convert
+// CHECK-VOL: fir.copy %{{.*}} to %{{.*}} no_overlap : !fir.ref<!fir.type<vol_t{i:i32}>>, !fir.ref<!fir.type<vol_t{i:i32}>, volatile>
+
+// Non-volatile LHS, volatile RHS.
+func.func @test_volatile_rhs(%a: !fir.ref<!t_vol>, %b: !fir.ref<!t_vol, volatile>) {
+ hlfir.assign %b to %a : !fir.ref<!t_vol, volatile>, !fir.ref<!t_vol>
+ return
+}
+// CHECK-VOL-LABEL: func.func @test_volatile_rhs(
+// CHECK-VOL-NOT: fir.convert
+// CHECK-VOL: fir.copy %{{.*}} to %{{.*}} no_overlap : !fir.ref<!fir.type<vol_t{i:i32}>, volatile>, !fir.ref<!fir.type<vol_t{i:i32}>>
+
+// Volatile LHS and RHS.
+func.func @test_volatile_both(%a: !fir.ref<!t_vol, volatile>, %b: !fir.ref<!t_vol, volatile>) {
+ hlfir.assign %b to %a : !fir.ref<!t_vol, volatile>, !fir.ref<!t_vol, volatile>
+ return
+}
+// CHECK-VOL-LABEL: func.func @test_volatile_both(
+// CHECK-VOL-NOT: fir.convert
+// CHECK-VOL: fir.copy %{{.*}} to %{{.*}} no_overlap : !fir.ref<!fir.type<vol_t{i:i32}>, volatile>, !fir.ref<!fir.type<vol_t{i:i32}>, volatile>
diff --git a/flang/test/Lower/Intrinsics/transfer.f90 b/flang/test/Lower/Intrinsics/transfer.f90
index a460ed5dfcb4e..b078a7c509fb3 100644
--- a/flang/test/Lower/Intrinsics/transfer.f90
+++ b/flang/test/Lower/Intrinsics/transfer.f90
@@ -1,4 +1,5 @@
! RUN: %flang_fc1 -emit-hlfir %s -o - | FileCheck %s
+! RUN: bbc -emit-fir %s -o - | FileCheck %s --check-prefix=FIR-CHECK
subroutine trans_test(store, word)
! CHECK-LABEL: func @_QPtrans_test(
@@ -162,6 +163,12 @@ subroutine trans_test_seq_tail_pad(raw, x)
! CHECK-NOT: hlfir.assign
! CHECK: return
! CHECK: }
+ ! FIR-CHECK-LABEL: func @_QPtrans_test_seq_tail_pad(
+ ! FIR-CHECK: fir.call @_FortranATransfer(
+ ! FIR-CHECK: fir.copy {{.*}} no_overlap : !fir.ref<!fir.type<_QFtrans_test_seq_tail_padTt,sequence{a:i32,b:i8}>>, !fir.ref<!fir.type<_QFtrans_test_seq_tail_padTt,sequence{a:i32,b:i8}>>
+ ! FIR-CHECK-NOT: fir.coordinate_of
+ ! FIR-CHECK: return
+ ! FIR-CHECK: }
use iso_c_binding, only: c_int, c_int8_t
type :: t
sequence
More information about the flang-commits
mailing list