[flang-commits] [flang] [flang] Inherit target specific code for BIND(C) types on Windows (PR #129579)

[Markus Mützel via flang-commits]

https://github.com/mmuetzel updated https://github.com/llvm/llvm-project/pull/129579

>From 0277f085f5eff8da7a9aad26dcc55e4e4f9c357b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20M=C3=BCtzel?= <markus.muetzel at gmx.de>
Date: Mon, 3 Mar 2025 10:32:22 +0100
Subject: [PATCH 1/2] [flang] Inherit target specific code for BIND(C) types on
 Windows

Inherit target specific code for Windows i386 and x86_64 from the classes that
define that code for the respective processors on non-Windows operating
systems.
Only overload parts that differ.

That allows re-using the existing implementation for BIND(C) types on
non-Windows x86_64 also for Windows x86_64 targets.

Fixes #114035.
---
 flang/lib/Optimizer/CodeGen/Target.cpp               | 12 ++++--------
 .../struct-passing-x86-64-several-fields-inreg.fir   |  1 +
 flang/test/Fir/target-rewrite-boxchar.fir            |  1 +
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/flang/lib/Optimizer/CodeGen/Target.cpp b/flang/lib/Optimizer/CodeGen/Target.cpp
index e2f8fb9d239a1..1c31e1b18f35c 100644
--- a/flang/lib/Optimizer/CodeGen/Target.cpp
+++ b/flang/lib/Optimizer/CodeGen/Target.cpp
@@ -199,10 +199,8 @@ struct TargetI386 : public GenericTarget<TargetI386> {
 //===----------------------------------------------------------------------===//
 
 namespace {
-struct TargetI386Win : public GenericTarget<TargetI386Win> {
-  using GenericTarget::GenericTarget;
-
-  static constexpr int defaultWidth = 32;
+struct TargetI386Win : public TargetI386 {
+  using TargetI386::TargetI386;
 
   CodeGenSpecifics::Marshalling
   complexArgumentType(mlir::Location loc, mlir::Type eleTy) const override {
@@ -718,10 +716,8 @@ struct TargetX86_64 : public GenericTarget<TargetX86_64> {
 //===----------------------------------------------------------------------===//
 
 namespace {
-struct TargetX86_64Win : public GenericTarget<TargetX86_64Win> {
-  using GenericTarget::GenericTarget;
-
-  static constexpr int defaultWidth = 64;
+struct TargetX86_64Win : public TargetX86_64 {
+  using TargetX86_64::TargetX86_64;
 
   CodeGenSpecifics::Marshalling
   complexArgumentType(mlir::Location loc, mlir::Type eleTy) const override {
diff --git a/flang/test/Fir/struct-passing-x86-64-several-fields-inreg.fir b/flang/test/Fir/struct-passing-x86-64-several-fields-inreg.fir
index 9a0a41e1da542..bd9f5fff44934 100644
--- a/flang/test/Fir/struct-passing-x86-64-several-fields-inreg.fir
+++ b/flang/test/Fir/struct-passing-x86-64-several-fields-inreg.fir
@@ -2,6 +2,7 @@
 // struct has more than one field.
 // REQUIRES: x86-registered-target
 // RUN: fir-opt -target-rewrite="target=x86_64-unknown-linux-gnu" %s -o - | FileCheck %s
+// RUN: fir-opt -target-rewrite="target=x86_64-w64-windows-gnu" %s -o - | FileCheck %s
 
 
 module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", 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", llvm.target_triple = "x86_64-unknown-linux-gnu"} {
diff --git a/flang/test/Fir/target-rewrite-boxchar.fir b/flang/test/Fir/target-rewrite-boxchar.fir
index 681536cde6a05..6583cde8820aa 100644
--- a/flang/test/Fir/target-rewrite-boxchar.fir
+++ b/flang/test/Fir/target-rewrite-boxchar.fir
@@ -1,5 +1,6 @@
 // RUN: fir-opt --target-rewrite="target=i386-unknown-linux-gnu" %s | FileCheck %s --check-prefix=INT32
 // RUN: fir-opt --target-rewrite="target=x86_64-unknown-linux-gnu" %s | FileCheck %s --check-prefix=INT64
+// RUN: fir-opt --target-rewrite="target=x86_64-w64-windows-gnu" %s | FileCheck %s --check-prefix=INT64
 // RUN: fir-opt --target-rewrite="target=aarch64-unknown-linux-gnu" %s | FileCheck %s --check-prefix=INT64
 // RUN: fir-opt --target-rewrite="target=powerpc64le-unknown-linux-gnu" %s | FileCheck %s --check-prefix=INT64
 // RUN: fir-opt --target-rewrite="target=amdgcn-amd-amdhsa" %s | FileCheck %s --check-prefix=INT64

>From 9ed20d3d9ce9d27f5805dc2e8ef92586312d121b Mon Sep 17 00:00:00 2001
From: jeanPerier <jperier at nvidia.com>
Date: Thu, 6 Mar 2025 18:05:33 +0100
Subject: [PATCH 2/2] [flang] Add struct passing for BIND(C) values for Windows
 x86_64

---
 flang/lib/Optimizer/CodeGen/Target.cpp | 39 ++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/flang/lib/Optimizer/CodeGen/Target.cpp b/flang/lib/Optimizer/CodeGen/Target.cpp
index 1c31e1b18f35c..f023be49cc053 100644
--- a/flang/lib/Optimizer/CodeGen/Target.cpp
+++ b/flang/lib/Optimizer/CodeGen/Target.cpp
@@ -776,6 +776,45 @@ struct TargetX86_64Win : public TargetX86_64 {
     }
     return marshal;
   }
+
+  CodeGenSpecifics::Marshalling
+  structArgumentType(mlir::Location loc, fir::RecordType type,
+                     const Marshalling &) const override {
+    std::uint64_t size =
+        fir::getTypeSizeAndAlignmentOrCrash(loc, type, getDataLayout(), kindMap)
+            .first;
+    CodeGenSpecifics::Marshalling marshal;
+    if (size <= 8)
+      marshal.emplace_back(mlir::IntegerType::get(type.getContext(), size * 8),
+                           AT{});
+    else
+      // TODO: investigate: clang is not setting the byval attribute, it is not
+      // clear why. Currently, this needs to be set here for flang so that the
+      // target-rewrite pass allocate memory as expected. Is it OK to set byval
+      // when clang does not?
+      marshal.emplace_back(fir::ReferenceType::get(type),
+                           AT{16, /*byval=*/true, /*sret=*/false});
+    return marshal;
+  }
+
+  CodeGenSpecifics::Marshalling
+  structReturnType(mlir::Location loc, fir::RecordType type) const override {
+    std::uint64_t size =
+        fir::getTypeSizeAndAlignmentOrCrash(loc, type, getDataLayout(), kindMap)
+            .first;
+    CodeGenSpecifics::Marshalling marshal;
+    if (size <= 8)
+      marshal.emplace_back(mlir::IntegerType::get(type.getContext(), size * 8),
+                           AT{});
+    else
+      // TODO: investigate: the ABI is not very clear about the alignment for
+      // the return in memory (while it was explicit about 16 for the argument
+      // passing case). Should it be the default alignment for that type
+      // instead?
+      marshal.emplace_back(fir::ReferenceType::get(type),
+                           AT{16, /*byval=*/false, /*sret=*/true});
+    return marshal;
+  }
 };
 } // namespace