[clang] [AMDGPU] Synthetic return coerce for aggregates with empty-for-layout members. (PR #197004)
@@ -21,6 +25,94 @@ using namespace clang::CodeGen;
namespace {
+/// True if \p Ty is a record whose fields (or bases) include a field that
abidh wrote:
Thanks for the review. I have added some more tests that cover additional code
paths. I am not sure we have a pre-existing code that exactly does what we
need. Please see the comments
[here](https://github.com/llvm/llvm-project/pull/96422#issuecomment-2887229576)
and
[here](https://github.com/llvm/llvm-project/pull/96422#issuecomment-2887305147)
which motivated this work.
https://github.com/llvm/llvm-project/pull/197004
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AMDGPU] Synthetic return coerce for aggregates with empty-for-layout members. (PR #197004)
https://github.com/abidh updated
https://github.com/llvm/llvm-project/pull/197004
>From 6ecc9ee71ee8b55a4c71c426e490b9b6d6dfa245 Mon Sep 17 00:00:00 2001
From: Abid Qadeer
Date: Wed, 29 Apr 2026 16:09:40 +0100
Subject: [PATCH 1/4] [AMDGPU] Synthetic return coerce for aggregates with
empty-for-layout members.
After llvm#96422, empty-for-layout members can show up as 4 x i8. This
logically empty type ends up consuming 4 VGPRs and breaks the ABI.
This PR teaches the AMDGPU ABI to use an explicit synthetic coerce
struct when it is returning a struct that can transitively contain an
empty-for-layout member. This coerce struct does not create array for
padding bytes. As a result, the fields go in the registers as ABI
expects. The numRegsForType has been fixed accordingly as well.
---
clang/lib/CodeGen/Targets/AMDGPU.cpp | 111 +-
.../amdgpu-aggregate-return-coerce.hip| 32 +
2 files changed, 142 insertions(+), 1 deletion(-)
create mode 100644 clang/test/CodeGenHIP/amdgpu-aggregate-return-coerce.hip
diff --git a/clang/lib/CodeGen/Targets/AMDGPU.cpp
b/clang/lib/CodeGen/Targets/AMDGPU.cpp
index a3a596bb9d822..06bd6076d4e50 100644
--- a/clang/lib/CodeGen/Targets/AMDGPU.cpp
+++ b/clang/lib/CodeGen/Targets/AMDGPU.cpp
@@ -9,7 +9,11 @@
#include "ABIInfoImpl.h"
#include "TargetInfo.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecordLayout.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/IR/DerivedTypes.h"
#include "llvm/Support/AMDGPUAddrSpace.h"
using namespace clang;
@@ -21,6 +25,94 @@ using namespace clang::CodeGen;
namespace {
+/// True if \p Ty is a record whose fields (or bases) include a field that
+/// is empty for layout, or that contain such a field transitively through
+/// member or base types.
+static bool recordTypeHasEmptyFieldForLayout(ASTContext &Ctx, QualType Ty) {
+ const RecordDecl *RD = Ty->getAsRecordDecl();
+ if (!RD)
+return false;
+
+ if (const auto *CXXRD = dyn_cast(RD)) {
+for (const auto &B : CXXRD->bases()) {
+ if (recordTypeHasEmptyFieldForLayout(Ctx, B.getType()))
+return true;
+}
+ }
+
+ for (const FieldDecl *FD : RD->fields()) {
+if (isEmptyFieldForLayout(Ctx, FD))
+ return true;
+if (recordTypeHasEmptyFieldForLayout(Ctx, FD->getType()))
+ return true;
+ }
+ return false;
+}
+
+/// Build a LLVM struct for AMDGPU aggregate return coercion: one element per
+/// non-empty base subobject and per field, ordered by \c ASTRecordLayout
+/// offsets (matching in-object layout). Nested records that also need this
+/// coercion use a nested coerce type; otherwise \c ConvertType is used.
+static llvm::Type *buildAMDGPUAggregateReturnCoerceType(CodeGenTypes &CGT,
+ASTContext &Ctx,
+QualType Ty) {
+ if (!recordTypeHasEmptyFieldForLayout(Ctx, Ty))
+return nullptr;
+
+ const RecordDecl *RD = Ty->getAsRecordDecl();
+ if (!RD || !RD->getDefinition() || RD->isUnion())
+return nullptr;
+ assert(!RD->hasFlexibleArrayMember());
+
+ // Vtable and dynamic-class layout are not represented here; use the normal
+ // LLVM record type as the coerce-to type.
+ if (const auto *CXXRD = dyn_cast(RD))
+if (CXXRD->isDynamicClass())
+ return nullptr;
+
+ const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
+
+ struct CoerceMember {
+CharUnits Offset;
+QualType Ty;
+ };
+ llvm::SmallVector Members;
+
+ if (const auto *CXXRD = dyn_cast(RD)) {
+for (const CXXBaseSpecifier &B : CXXRD->bases()) {
+ const CXXRecordDecl *BaseDecl = B.getType()->getAsCXXRecordDecl();
+ if (!BaseDecl || BaseDecl->isEmpty())
+continue;
+ BaseDecl = BaseDecl->getDefinition();
+ CharUnits Off = B.isVirtual() ? Layout.getVBaseClassOffset(BaseDecl)
+: Layout.getBaseClassOffset(BaseDecl);
+ Members.push_back({Off, B.getType()});
+}
+ }
+
+ for (const FieldDecl *FD : RD->fields()) {
+CharUnits Off =
+Ctx.toCharUnitsFromBits(Layout.getFieldOffset(FD->getFieldIndex()));
+Members.push_back({Off, FD->getType()});
+ }
+
+ llvm::stable_sort(Members, [](const CoerceMember &A, const CoerceMember &B) {
+return A.Offset < B.Offset;
+ });
+
+ llvm::LLVMContext &VM = CGT.getLLVMContext();
+ llvm::SmallVector Elts;
+ for (const CoerceMember &M : Members) {
+if (llvm::Type *Nested =
+buildAMDGPUAggregateReturnCoerceType(CGT, Ctx, M.Ty))
+ Elts.push_back(Nested);
+else
+ Elts.push_back(CGT.ConvertType(M.Ty));
+ }
+
+ return llvm::StructType::create(VM, Elts);
+}
+
class AMDGPUABIInfo final : public DefaultABIInfo {
private:
static const unsigned MaxNumRegsForArgsRet = 16;
@@ -99,7 +191,20 @@ uint64_t AMDGPUABIInfo::numRegsForType(QualType Ty) const {
if (const auto *RD = Ty->getAsRe
[clang] [AMDGPU] Synthetic return coerce for aggregates with empty-for-layout members. (PR #197004)
@@ -21,6 +25,94 @@ using namespace clang::CodeGen;
namespace {
+/// True if \p Ty is a record whose fields (or bases) include a field that
+/// is empty for layout, or that contain such a field transitively through
+/// member or base types.
+static bool recordTypeHasEmptyFieldForLayout(ASTContext &Ctx, QualType Ty) {
+ const RecordDecl *RD = Ty->getAsRecordDecl();
+ if (!RD)
+return false;
+
+ if (const auto *CXXRD = dyn_cast(RD)) {
+for (const auto &B : CXXRD->bases()) {
+ if (recordTypeHasEmptyFieldForLayout(Ctx, B.getType()))
+return true;
+}
+ }
+
+ for (const FieldDecl *FD : RD->fields()) {
+if (isEmptyFieldForLayout(Ctx, FD))
+ return true;
+if (recordTypeHasEmptyFieldForLayout(Ctx, FD->getType()))
+ return true;
+ }
+ return false;
+}
+
+/// Build a LLVM struct for AMDGPU aggregate return coercion: one element per
+/// non-empty base subobject and per field, ordered by \c ASTRecordLayout
+/// offsets (matching in-object layout). Nested records that also need this
+/// coercion use a nested coerce type; otherwise \c ConvertType is used.
+static llvm::Type *buildAMDGPUAggregateReturnCoerceType(CodeGenTypes &CGT,
+ASTContext &Ctx,
+QualType Ty) {
+ if (!recordTypeHasEmptyFieldForLayout(Ctx, Ty))
+return nullptr;
+
+ const RecordDecl *RD = Ty->getAsRecordDecl();
+ if (!RD || !RD->getDefinition() || RD->isUnion())
+return nullptr;
+ assert(!RD->hasFlexibleArrayMember());
+
+ // Vtable and dynamic-class layout are not represented here; use the normal
+ // LLVM record type as the coerce-to type.
+ if (const auto *CXXRD = dyn_cast(RD))
+if (CXXRD->isDynamicClass())
+ return nullptr;
+
+ const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
+
+ struct CoerceMember {
+CharUnits Offset;
+QualType Ty;
+ };
+ llvm::SmallVector Members;
+
+ if (const auto *CXXRD = dyn_cast(RD)) {
+for (const CXXBaseSpecifier &B : CXXRD->bases()) {
+ const CXXRecordDecl *BaseDecl = B.getType()->getAsCXXRecordDecl();
+ if (!BaseDecl || BaseDecl->isEmpty())
+continue;
+ BaseDecl = BaseDecl->getDefinition();
+ CharUnits Off = B.isVirtual() ? Layout.getVBaseClassOffset(BaseDecl)
arsenm wrote:
In particular virtual base seems untested?
https://github.com/llvm/llvm-project/pull/197004
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AMDGPU] Synthetic return coerce for aggregates with empty-for-layout members. (PR #197004)
@@ -21,6 +25,94 @@ using namespace clang::CodeGen;
namespace {
+/// True if \p Ty is a record whose fields (or bases) include a field that
arsenm wrote:
I mean that the amount of code to number of test cases is very high. I would
also expect that skip-nop-fields is something that would some pre-existing
generic code to handle
https://github.com/llvm/llvm-project/pull/197004
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AMDGPU] Synthetic return coerce for aggregates with empty-for-layout members. (PR #197004)
@@ -21,6 +25,94 @@ using namespace clang::CodeGen;
namespace {
+/// True if \p Ty is a record whose fields (or bases) include a field that
abidh wrote:
Can you kindly elaborate a bit more? Do you think that it is too much code to
handle a corner case. I am open to any other approaches for this problem.
https://github.com/llvm/llvm-project/pull/197004
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AMDGPU] Synthetic return coerce for aggregates with empty-for-layout members. (PR #197004)
https://github.com/maarquitos14 approved this pull request. I think this is fine for SPIRV as long as the roundtrip (LLVM->SPIRV->LLVM) keeps the synthetic coerce struct, because then that would be passed to AMDGCN to take care of it as expected. https://github.com/llvm/llvm-project/pull/197004 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
