[flang-commits] [flang] [flang] Fix `getTypeSizeAndAlignment` for packed/tail-padded `RecordType` and `TRANSFER` gate (PR #220377)

[Daniel Chen via flang-commits]

================
@@ -8972,22 +8972,64 @@ IntrinsicLibrary::genTransfer(mlir::Type resultType,
         (fir::isa_trivial(sourceType) ||
          mlir::isa<fir::RecordType>(sourceType)) &&
         fir::isa_trivial(moldType)) {
+      // Compare store sizes (data bytes, no tail padding) because Fortran
+      // TRANSFER is defined over STORAGE_SIZE data bits.  For a RecordType
+      // source, storeSizeOnly=true strips the tail padding that the compiler
+      // adds for alignment but that is not part of the type's data.
+      // Note: the runtime (TransferImpl) copies source.ElementBytes() bytes,
+      // which is the allocation size and may include tail padding.  The store-
+      // size comparison here is therefore *not* about matching the runtime's
+      // copy width; it is about selecting the correct inline path according to
+      // the standard's data-bit definition.
+      //
+      // Alignment is a separate concern: when the source alignment is less
+      // than the result type's alignment, the RecordType path below copies
+      // into a result-aligned alloca rather than loading directly from the
+      // source pointer.
       auto sourceSizeAndAlign = fir::getTypeSizeAndAlignment(
-          loc, sourceType, builder.getDataLayout(), builder.getKindMap());
+          loc, sourceType, builder.getDataLayout(), builder.getKindMap(),
+          /*storeSizeOnly=*/true);
       auto resultSizeAndAlign = fir::getTypeSizeAndAlignment(
-          loc, resultType, builder.getDataLayout(), builder.getKindMap());
+          loc, resultType, builder.getDataLayout(), builder.getKindMap(),
+          /*storeSizeOnly=*/true);
       if (sourceSizeAndAlign && resultSizeAndAlign &&
           sourceSizeAndAlign->first == resultSizeAndAlign->first) {
-        if (sourceType.isSignlessIntOrFloat() &&
-            resultType.isSignlessIntOrFloat()) {
-          mlir::Value val = fir::LoadOp::create(builder, loc, sourceBase);
-          if (sourceType != resultType)
-            val = mlir::arith::BitcastOp::create(builder, loc, resultType, val);
-          return val;
+        if (fir::isa_trivial(sourceType)) {
+          // Both source and result are trivial scalars of the same store
+          // size.  Use arith.bitcast for signless integer/float pairs;
+          // for other trivial types (e.g. unsigned integers) arith.bitcast
+          // is not available, so cast the source address and load.
+          if (sourceType.isSignlessIntOrFloat() &&
+              resultType.isSignlessIntOrFloat()) {
+            mlir::Value val = fir::LoadOp::create(builder, loc, sourceBase);
+            if (sourceType != resultType)
+              val =
+                  mlir::arith::BitcastOp::create(builder, loc, resultType, val);
+            return val;
+          }
+          mlir::Type refTy = builder.getRefType(resultType);
+          mlir::Value cast = builder.createConvert(loc, refTy, sourceBase);
+          return fir::LoadOp::create(builder, loc, cast);
+        }
+        // The source is a RecordType.  When the source ABI alignment is
+        // sufficient for resultType, a direct address cast and load is safe.
+        // When sourceAlign < resultAlign (e.g. {i64,i16} is 8-byte aligned
+        // while x86_fp80 requires 16-byte alignment), loading resultType
+        // directly from sourceBase would claim an over-aligned address,
+        // producing undefined behaviour.  In that case copy the bytes into
+        // a result-typed alloca (which receives resultType's natural
+        // alignment) and load from there.
+        if (sourceSizeAndAlign->second >= resultSizeAndAlign->second) {
+          mlir::Type refTy = builder.getRefType(resultType);
+          mlir::Value cast = builder.createConvert(loc, refTy, sourceBase);
+          return fir::LoadOp::create(builder, loc, cast);
         }
-        mlir::Type refTy = builder.getRefType(resultType);
-        mlir::Value cast = builder.createConvert(loc, refTy, sourceBase);
-        return fir::LoadOp::create(builder, loc, cast);
+        mlir::Value tmp = fir::AllocaOp::create(builder, loc, resultType);
+        mlir::Value srcLoad = fir::LoadOp::create(builder, loc, sourceBase);
+        mlir::Type srcRefTy = builder.getRefType(sourceType);
+        mlir::Value tmpAsSrc = builder.createConvert(loc, srcRefTy, tmp);
+        fir::StoreOp::create(builder, loc, srcLoad, tmpAsSrc);
----------------
DanielCChen wrote:

Updated `TRANSFER` to use a byte-wise `fir.copy` into aligned storage, preserving internal padding bytes.  
Added regression coverage for a padded `BIND(C)` record transferred to `INTEGER(16)`.

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