[flang-commits] [flang] [flang] Fix TRANSFER into derived type with tail padding (PR #223814)
Daniel Chen via flang-commits
flang-commits at lists.llvm.org
Sat Sep 19 12:34:12 PDT 2026
https://github.com/DanielCChen updated https://github.com/llvm/llvm-project/pull/223814
>From c086924669326af1fbf557913be7c048e20377d3 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/4] [flang] Fix TRANSFER into derived type with tail padding
---
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 c6e16ea56214dbb0dd57cf6c782438ff01275be6 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/4] [flang] Fix volatile, no_overlap, and CUF 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: expand fir.copy uses of a predefined-variable
fir.declare into per-field NVVM register reads + stores, so a
whole-record assignment like `idx = threadIdx` fills the destination
with actual GPU register values rather than copying from the global
backing variable. The declare and the global are erased once all uses
are handled; fir.call and other unrecognised uses keep the declare alive.
- Tests: add volatile assignment cases to assign-codegen-derived.fir,
a self-assignment case (no no_overlap) 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 an expanded fir.copy 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
>From 09eaf2166763d20003c8d95c102273023d859b3a Mon Sep 17 00:00:00 2001
From: Daniel Chen <cdchen at ca.ibm.com>
Date: Fri, 18 Sep 2026 10:21:53 -0400
Subject: [PATCH 3/4] [flang] Expand predefined-var fir.copy into NVVM reads;
add no_overlap test
- CUFPredefinedVarToGPU: replace the skip-and-survive guard for fir.copy
uses of a predefined-variable fir.declare with a full expansion: emit
three per-field NVVM register reads + stores so that a whole-record
assignment like `idx = threadIdx` fills the destination with actual GPU
register values rather than copying from the global backing variable.
The declare and the global are erased once all uses are handled.
- assign-codegen-derived.fir: add test_simple_self_assign to cover the
noOverlap=false path -- a self-assignment (lhs == rhs) causes alias
analysis to return MustAlias, so fir.copy is emitted without no_overlap
(lowering to memmove rather than memcpy).
- predefined-variables.mlir: update the fir.copy test case to assert the
expanded form: no fir.copy or declare survives, all three fields are
written from NVVM tid reads, and the subsequent component read is also
lowered to a register read.
---
.../Transforms/CUDA/CUFPredefinedVarToGPU.cpp | 72 ++++++++++++++-----
flang/test/Fir/CUDA/predefined-variables.mlir | 35 ++++++---
flang/test/HLFIR/assign-codegen-derived.fir | 15 ++++
3 files changed, 93 insertions(+), 29 deletions(-)
diff --git a/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp b/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp
index 8c49fc8e9ce7b..1b380aedd7539 100644
--- a/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp
+++ b/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp
@@ -67,6 +67,29 @@ processCoordinateOp(mlir::OpBuilder &builder, fir::CoordinateOp coordOp,
}
}
+// Emit one NVVM register read for field `fieldIdx` of `dest` and store the
+// result there. Used to expand a whole-record assignment such as
+// `idx = threadIdx` into three per-field register reads so the destination
+// receives actual GPU register values rather than a copy from the global.
+template <typename OpTy>
+static void emitFieldStore(mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::Value dest, unsigned fieldIdx,
+ bool incrementByOne) {
+ mlir::Type i32Ty = builder.getI32Type();
+ mlir::Value gpuVal = OpTy::create(builder, loc, i32Ty);
+ if (incrementByOne) {
+ auto c1 = mlir::arith::ConstantOp::create(builder, loc, i32Ty,
+ builder.getI32IntegerAttr(1));
+ gpuVal = mlir::arith::AddIOp::create(builder, loc, gpuVal, c1);
+ }
+ fir::IntOrValue idx =
+ mlir::IntegerAttr::get(i32Ty, static_cast<int32_t>(fieldIdx));
+ mlir::Value fieldRef = fir::CoordinateOp::create(
+ builder, loc, fir::ReferenceType::get(i32Ty), dest,
+ llvm::SmallVector<fir::IntOrValue, 1>{idx});
+ fir::StoreOp::create(builder, loc, gpuVal, fieldRef);
+}
+
template <typename OpTyX, typename OpTyY, typename OpTyZ>
static void
processDeclareOp(mlir::OpBuilder &builder, fir::DeclareOp declareOp,
@@ -74,26 +97,39 @@ 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;
+ // Snapshot uses before queuing deletions to avoid invalidating the
+ // iterator as ops are marked for removal.
+ llvm::SmallVector<mlir::Operation *> useOps;
+ for (mlir::OpOperand &use : declareOp.getResult().getUses())
+ useOps.push_back(use.getOwner());
+
+ bool allUsesHandled = true;
+ for (mlir::Operation *useOp : useOps) {
+ if (auto coordOp = mlir::dyn_cast<fir::CoordinateOp>(useOp)) {
+ processCoordinateOp<OpTyX>(builder, coordOp, field_x, incrementByOne,
+ opsToDelete);
+ processCoordinateOp<OpTyY>(builder, coordOp, field_y, incrementByOne,
+ opsToDelete);
+ processCoordinateOp<OpTyZ>(builder, coordOp, field_z, incrementByOne,
+ opsToDelete);
+ opsToDelete.push_back(coordOp);
+ } else if (auto copyOp = mlir::dyn_cast<fir::CopyOp>(useOp)) {
+ // Whole-record assignment (e.g. `idx = threadIdx`): expand into three
+ // per-field NVVM reads so the destination receives GPU register values
+ // rather than a copy from the global backing variable.
+ mlir::Value dest = copyOp.getDestination();
+ mlir::Location loc = copyOp.getLoc();
+ builder.setInsertionPoint(copyOp);
+ emitFieldStore<OpTyX>(builder, loc, dest, field_x, incrementByOne);
+ emitFieldStore<OpTyY>(builder, loc, dest, field_y, incrementByOne);
+ emitFieldStore<OpTyZ>(builder, loc, dest, field_z, incrementByOne);
+ opsToDelete.push_back(copyOp);
+ } else {
+ // Other uses (e.g. fir.call): leave in place, keep the declare alive.
+ allUsesHandled = false;
}
- processCoordinateOp<OpTyX>(builder, coordOp, field_x, incrementByOne,
- opsToDelete);
- processCoordinateOp<OpTyY>(builder, coordOp, field_y, incrementByOne,
- opsToDelete);
- processCoordinateOp<OpTyZ>(builder, coordOp, field_z, incrementByOne,
- opsToDelete);
- opsToDelete.push_back(coordOp);
}
- if (allUsesAreCoords)
+ if (allUsesHandled)
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).
diff --git a/flang/test/Fir/CUDA/predefined-variables.mlir b/flang/test/Fir/CUDA/predefined-variables.mlir
index ce4af29f4be73..dbc9f9d962d26 100644
--- a/flang/test/Fir/CUDA/predefined-variables.mlir
+++ b/flang/test/Fir/CUDA/predefined-variables.mlir
@@ -495,9 +495,10 @@ func.func @_QPsub4(%arg0: !fir.ref<i32> {fir.bindc_name = "i", cuf.data_attr = #
// -----
// 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.
+// assignment like `idx = threadIdx`) must be expanded into per-field NVVM
+// register reads so the destination receives actual GPU register values rather
+// than a copy from the global backing storage. Both the fir.copy and the
+// fir.declare are removed once all uses have been rewritten.
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}>>
@@ -512,12 +513,24 @@ func.func @_QPcopy_threadidx(%arg0: !fir.ref<!fir.type<_QM__fortran_builtinsT__b
return
}
-// The declare must survive (fir.copy still uses it).
+// The fir.copy is replaced by three per-field NVVM reads into the destination;
+// the declare and the global backing variable are both removed.
// 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>
+// CHECK-NOT: _QM__fortran_builtinsE__builtin_threadidx
+// CHECK-NOT: fir.copy
+// Whole-record copy expanded: x field (field 0).
+// CHECK: %[[TIDX:.*]] = nvvm.read.ptx.sreg.tid.x : i32
+// CHECK: %[[ADDX:.*]] = arith.addi %[[TIDX]], %c1{{.*}} : i32
+// CHECK: fir.store %[[ADDX]] to %{{.*}} : !fir.ref<i32>
+// Whole-record copy expanded: y field (field 1).
+// CHECK: %[[TIDY:.*]] = nvvm.read.ptx.sreg.tid.y : i32
+// CHECK: %[[ADDY:.*]] = arith.addi %[[TIDY]], %c1{{.*}} : i32
+// CHECK: fir.store %[[ADDY]] to %{{.*}} : !fir.ref<i32>
+// Whole-record copy expanded: z field (field 2).
+// CHECK: %[[TIDZ:.*]] = nvvm.read.ptx.sreg.tid.z : i32
+// CHECK: %[[ADDZ:.*]] = arith.addi %[[TIDZ]], %c1{{.*}} : i32
+// CHECK: fir.store %[[ADDZ]] to %{{.*}} : !fir.ref<i32>
+// The subsequent component read (i = threadIdx%x) also becomes a register read.
+// CHECK: %[[TID2:.*]] = nvvm.read.ptx.sreg.tid.x : i32
+// CHECK: %[[ADD2:.*]] = arith.addi %[[TID2]], %c1{{.*}} : i32
+// CHECK: fir.store %[[ADD2]] to %{{.*}} : !fir.ref<i32>
diff --git a/flang/test/HLFIR/assign-codegen-derived.fir b/flang/test/HLFIR/assign-codegen-derived.fir
index 64533a423f448..305ced16c0ee7 100644
--- a/flang/test/HLFIR/assign-codegen-derived.fir
+++ b/flang/test/HLFIR/assign-codegen-derived.fir
@@ -76,3 +76,18 @@ func.func @test_volatile_both(%a: !fir.ref<!t_vol, volatile>, %b: !fir.ref<!t_vo
// 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>
+
+
+// Non-SEQUENCE derived type self-assignment: lhs == rhs so alias analysis
+// returns MustAlias. noOverlap requires isTemporaryLHS OR NoAlias; neither
+// holds here, so fir.copy is emitted WITHOUT no_overlap (lowers to memmove).
+// disjoint is still true for non-SEQUENCE types so we take the fir.copy fast
+// path rather than the _FortranAAssign runtime path.
+func.func @test_simple_self_assign(%a: !fir.ref<!t_simple>) {
+ hlfir.assign %a to %a : !fir.ref<!t_simple>, !fir.ref<!t_simple>
+ return
+}
+// CHECK-LABEL: func.func @test_simple_self_assign(
+// CHECK-NOT: fir.coordinate_of
+// CHECK: fir.copy %{{.*}} to %{{.*}} : !fir.ref<!fir.type<simple{i:i32}>>, !fir.ref<!fir.type<simple{i:i32}>>
+// CHECK-NOT: no_overlap
>From ecb32307a4b35fc4bd04f282c4f471b8409a1a2d Mon Sep 17 00:00:00 2001
From: Daniel Chen <cdchen at ca.ibm.com>
Date: Sat, 19 Sep 2026 15:33:43 -0400
Subject: [PATCH 4/4] [flang][CUF] Preserve volatility in whole-record
threadIdx expansion
When CUFPredefinedVarToGPU expands a fir.copy whose destination is a
volatile dim3 variable (e.g. `type(dim3), volatile :: idx; idx = threadIdx`)
into three per-field NVVM register reads, the emitted fir.coordinate_of
ops were always typed !fir.ref<i32> regardless of the destination's
volatility. This caused the resulting fir.store ops to be non-volatile,
dropping the memory effect and failing --strict-fir-volatile-verifier.
Fix emitFieldStore to query fir::isa_volatile_type on the destination
and pass the flag to fir::ReferenceType::get, so a volatile destination
produces !fir.ref<i32, volatile> field references and volatile stores.
Add a test case in predefined-variables.mlir covering a volatile dim3
destination, with CHECK lines that capture the destination declare and
verify each field coordinate and store carry the volatile qualifier.
Also tighten the self-assignment check in assign-codegen-derived.fir:
match fir.copy %arg0 to %arg0 : ... directly (the fir.declare is folded
away) so the check rejects any form carrying no_overlap.
---
.../Transforms/CUDA/CUFPredefinedVarToGPU.cpp | 6 ++-
flang/test/Fir/CUDA/predefined-variables.mlir | 50 +++++++++++++++++--
flang/test/HLFIR/assign-codegen-derived.fir | 2 +-
3 files changed, 53 insertions(+), 5 deletions(-)
diff --git a/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp b/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp
index 1b380aedd7539..bc34e4ed66782 100644
--- a/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp
+++ b/flang/lib/Optimizer/Transforms/CUDA/CUFPredefinedVarToGPU.cpp
@@ -71,6 +71,9 @@ processCoordinateOp(mlir::OpBuilder &builder, fir::CoordinateOp coordOp,
// result there. Used to expand a whole-record assignment such as
// `idx = threadIdx` into three per-field register reads so the destination
// receives actual GPU register values rather than a copy from the global.
+// The field reference inherits the destination's volatility so that a volatile
+// destination (e.g. `type(dim3), volatile :: idx`) produces volatile stores
+// and passes --strict-fir-volatile-verifier.
template <typename OpTy>
static void emitFieldStore(mlir::OpBuilder &builder, mlir::Location loc,
mlir::Value dest, unsigned fieldIdx,
@@ -84,8 +87,9 @@ static void emitFieldStore(mlir::OpBuilder &builder, mlir::Location loc,
}
fir::IntOrValue idx =
mlir::IntegerAttr::get(i32Ty, static_cast<int32_t>(fieldIdx));
+ bool isVolatile = fir::isa_volatile_type(dest.getType());
mlir::Value fieldRef = fir::CoordinateOp::create(
- builder, loc, fir::ReferenceType::get(i32Ty), dest,
+ builder, loc, fir::ReferenceType::get(i32Ty, isVolatile), dest,
llvm::SmallVector<fir::IntOrValue, 1>{idx});
fir::StoreOp::create(builder, loc, gpuVal, fieldRef);
}
diff --git a/flang/test/Fir/CUDA/predefined-variables.mlir b/flang/test/Fir/CUDA/predefined-variables.mlir
index dbc9f9d962d26..ece593e04b642 100644
--- a/flang/test/Fir/CUDA/predefined-variables.mlir
+++ b/flang/test/Fir/CUDA/predefined-variables.mlir
@@ -518,19 +518,63 @@ func.func @_QPcopy_threadidx(%arg0: !fir.ref<!fir.type<_QM__fortran_builtinsT__b
// CHECK-LABEL: func.func @_QPcopy_threadidx
// CHECK-NOT: _QM__fortran_builtinsE__builtin_threadidx
// CHECK-NOT: fir.copy
+// Capture the destination declare so we can require each value lands in the
+// correct field.
+// CHECK: %[[DEST:.*]] = fir.declare %arg0 {uniq_name = "_QFsub1Eidx"}
// Whole-record copy expanded: x field (field 0).
// CHECK: %[[TIDX:.*]] = nvvm.read.ptx.sreg.tid.x : i32
// CHECK: %[[ADDX:.*]] = arith.addi %[[TIDX]], %c1{{.*}} : i32
-// CHECK: fir.store %[[ADDX]] to %{{.*}} : !fir.ref<i32>
+// CHECK: %[[COORDX:.*]] = fir.coordinate_of %[[DEST]], x :
+// CHECK: fir.store %[[ADDX]] to %[[COORDX]] : !fir.ref<i32>
// Whole-record copy expanded: y field (field 1).
// CHECK: %[[TIDY:.*]] = nvvm.read.ptx.sreg.tid.y : i32
// CHECK: %[[ADDY:.*]] = arith.addi %[[TIDY]], %c1{{.*}} : i32
-// CHECK: fir.store %[[ADDY]] to %{{.*}} : !fir.ref<i32>
+// CHECK: %[[COORDY:.*]] = fir.coordinate_of %[[DEST]], y :
+// CHECK: fir.store %[[ADDY]] to %[[COORDY]] : !fir.ref<i32>
// Whole-record copy expanded: z field (field 2).
// CHECK: %[[TIDZ:.*]] = nvvm.read.ptx.sreg.tid.z : i32
// CHECK: %[[ADDZ:.*]] = arith.addi %[[TIDZ]], %c1{{.*}} : i32
-// CHECK: fir.store %[[ADDZ]] to %{{.*}} : !fir.ref<i32>
+// CHECK: %[[COORDZ:.*]] = fir.coordinate_of %[[DEST]], z :
+// CHECK: fir.store %[[ADDZ]] to %[[COORDZ]] : !fir.ref<i32>
// The subsequent component read (i = threadIdx%x) also becomes a register read.
// CHECK: %[[TID2:.*]] = nvvm.read.ptx.sreg.tid.x : i32
// CHECK: %[[ADD2:.*]] = arith.addi %[[TID2]], %c1{{.*}} : i32
// CHECK: fir.store %[[ADD2]] to %{{.*}} : !fir.ref<i32>
+
+// -----
+
+// A fir.copy with a volatile destination (e.g. `type(dim3), volatile :: idx`)
+// must be expanded into stores through volatile field references so that the
+// memory effects are preserved and the output passes
+// --strict-fir-volatile-verifier.
+func.func @_QPcopy_volatile_threadidx(%arg0: !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>, volatile> {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 = "_QFcopy_volatile_threadidxEidx"} : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>, volatile>) -> !fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>, volatile>
+ // Whole-record assignment to a volatile destination: 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}>, volatile>
+ return
+}
+
+// The expanded stores must use volatile field references so that volatility is
+// preserved through the rewrite.
+// CHECK-LABEL: func.func @_QPcopy_volatile_threadidx
+// CHECK-NOT: _QM__fortran_builtinsE__builtin_threadidx
+// CHECK-NOT: fir.copy
+// Capture the volatile destination declare.
+// CHECK: %[[DEST:.*]] = fir.declare %arg0 {uniq_name = "_QFcopy_volatile_threadidxEidx"}
+// x field: store through a volatile field reference.
+// CHECK: %[[TIDX:.*]] = nvvm.read.ptx.sreg.tid.x : i32
+// CHECK: %[[ADDX:.*]] = arith.addi %[[TIDX]], %c1{{.*}} : i32
+// CHECK: %[[COORDX:.*]] = fir.coordinate_of %[[DEST]], x : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>, volatile>) -> !fir.ref<i32, volatile>
+// CHECK: fir.store %[[ADDX]] to %[[COORDX]] : !fir.ref<i32, volatile>
+// y field.
+// CHECK: %[[TIDY:.*]] = nvvm.read.ptx.sreg.tid.y : i32
+// CHECK: %[[ADDY:.*]] = arith.addi %[[TIDY]], %c1{{.*}} : i32
+// CHECK: %[[COORDY:.*]] = fir.coordinate_of %[[DEST]], y : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>, volatile>) -> !fir.ref<i32, volatile>
+// CHECK: fir.store %[[ADDY]] to %[[COORDY]] : !fir.ref<i32, volatile>
+// z field.
+// CHECK: %[[TIDZ:.*]] = nvvm.read.ptx.sreg.tid.z : i32
+// CHECK: %[[ADDZ:.*]] = arith.addi %[[TIDZ]], %c1{{.*}} : i32
+// CHECK: %[[COORDZ:.*]] = fir.coordinate_of %[[DEST]], z : (!fir.ref<!fir.type<_QM__fortran_builtinsT__builtin_dim3{x:i32,y:i32,z:i32}>, volatile>) -> !fir.ref<i32, volatile>
+// CHECK: fir.store %[[ADDZ]] to %[[COORDZ]] : !fir.ref<i32, volatile>
diff --git a/flang/test/HLFIR/assign-codegen-derived.fir b/flang/test/HLFIR/assign-codegen-derived.fir
index 305ced16c0ee7..0e01a4000fa55 100644
--- a/flang/test/HLFIR/assign-codegen-derived.fir
+++ b/flang/test/HLFIR/assign-codegen-derived.fir
@@ -89,5 +89,5 @@ func.func @test_simple_self_assign(%a: !fir.ref<!t_simple>) {
}
// CHECK-LABEL: func.func @test_simple_self_assign(
// CHECK-NOT: fir.coordinate_of
-// CHECK: fir.copy %{{.*}} to %{{.*}} : !fir.ref<!fir.type<simple{i:i32}>>, !fir.ref<!fir.type<simple{i:i32}>>
+// CHECK: fir.copy %arg0 to %arg0 : !fir.ref<!fir.type<simple{i:i32}>>, !fir.ref<!fir.type<simple{i:i32}>>
// CHECK-NOT: no_overlap
More information about the flang-commits
mailing list