Skip to content

[clang][SPARC] Pass 16-aligned structs with the correct alignment in CC #155829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 26, 2025
Merged

[clang][SPARC] Pass 16-aligned structs with the correct alignment in CC #155829

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a8707a6
[clang][SPARC] Pass 16-aligned 16-byte structs as i128 in CC
koachan Aug 28, 2025
05f7e2b
Fix comment and size checks
koachan Aug 28, 2025
ba36452
Add medium_aligned test
koachan Sep 4, 2025
fcfb1aa
New impl and add tests
koachan Sep 14, 2025
fffdefb
Link RetOffset with ArgOffset in case of indirect returns
koachan Sep 17, 2025
60744a6
Remove unused includes
koachan Sep 17, 2025
364730f
Use emitVoidPtrVAArg to implement va_arg
koachan Sep 20, 2025
58fd500
Add test for overaligned va_arg
koachan Sep 20, 2025
a11d8ae
Fix overaligned va_arg test case
koachan Sep 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 48 additions & 23 deletions clang/lib/CodeGen/Targets/Sparc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ class SparcV9ABIInfo : public ABIInfo {
SparcV9ABIInfo(CodeGenTypes &CGT) : ABIInfo(CGT) {}

private:
ABIArgInfo classifyType(QualType RetTy, unsigned SizeLimit) const;
ABIArgInfo classifyType(QualType RetTy, unsigned SizeLimit,
unsigned &RegOffset) const;
void computeInfo(CGFunctionInfo &FI) const override;
RValue EmitVAArg(CodeGenFunction &CGF, Address VAListAddr, QualType Ty,
AggValueSlot Slot) const override;
Expand Down Expand Up @@ -222,73 +223,94 @@ class SparcV9ABIInfo : public ABIInfo {
};
} // end anonymous namespace

ABIArgInfo
SparcV9ABIInfo::classifyType(QualType Ty, unsigned SizeLimit) const {
ABIArgInfo SparcV9ABIInfo::classifyType(QualType Ty, unsigned SizeLimit,
unsigned &RegOffset) const {
if (Ty->isVoidType())
return ABIArgInfo::getIgnore();

uint64_t Size = getContext().getTypeSize(Ty);
unsigned Alignment = getContext().getTypeAlign(Ty);
auto &Context = getContext();
auto &VMContext = getVMContext();

uint64_t Size = Context.getTypeSize(Ty);
unsigned Alignment = Context.getTypeAlign(Ty);
bool NeedPadding = (Alignment > 64) && (RegOffset % 2 != 0);

// Anything too big to fit in registers is passed with an explicit indirect
// pointer / sret pointer.
if (Size > SizeLimit)
if (Size > SizeLimit) {
RegOffset += 1;
return getNaturalAlignIndirect(
Ty, /*AddrSpace=*/getDataLayout().getAllocaAddrSpace(),
/*ByVal=*/false);
}

// Treat an enum type as its underlying type.
if (const auto *ED = Ty->getAsEnumDecl())
Ty = ED->getIntegerType();

// Integer types smaller than a register are extended.
if (Size < 64 && Ty->isIntegerType())
if (Size < 64 && Ty->isIntegerType()) {
RegOffset += 1;
return ABIArgInfo::getExtend(Ty);
}

if (const auto *EIT = Ty->getAs<BitIntType>())
if (EIT->getNumBits() < 64)
if (EIT->getNumBits() < 64) {
RegOffset += 1;
return ABIArgInfo::getExtend(Ty);
}

// Other non-aggregates go in registers.
if (!isAggregateTypeForABI(Ty))
if (!isAggregateTypeForABI(Ty)) {
RegOffset += Size / 64;
return ABIArgInfo::getDirect();
}

// If a C++ object has either a non-trivial copy constructor or a non-trivial
// destructor, it is passed with an explicit indirect pointer / sret pointer.
if (CGCXXABI::RecordArgABI RAA = getRecordArgABI(Ty, getCXXABI()))
if (CGCXXABI::RecordArgABI RAA = getRecordArgABI(Ty, getCXXABI())) {
RegOffset += 1;
return getNaturalAlignIndirect(Ty, getDataLayout().getAllocaAddrSpace(),
RAA == CGCXXABI::RAA_DirectInMemory);
}

// This is a small aggregate type that should be passed in registers.
// Build a coercion type from the LLVM struct type.
llvm::StructType *StrTy = dyn_cast<llvm::StructType>(CGT.ConvertType(Ty));
if (!StrTy)
if (!StrTy) {
RegOffset += Size / 64;
return ABIArgInfo::getDirect();
}

CoerceBuilder CB(getVMContext(), getDataLayout());
CoerceBuilder CB(VMContext, getDataLayout());
CB.addStruct(0, StrTy);
// All structs, even empty ones, should take up a register argument slot,
// so pin the minimum struct size to one bit.
CB.pad(llvm::alignTo(
std::max(CB.DL.getTypeSizeInBits(StrTy).getKnownMinValue(), uint64_t(1)),
64));
RegOffset += CB.Size / 64;

// If we're dealing with overaligned structs we may need to add a padding in
// the front, to preserve the correct register-memory mapping.
//
// See SCD 2.4.1, pages 3P-11 and 3P-12.
llvm::Type *Padding =
NeedPadding ? llvm::Type::getInt64Ty(VMContext) : nullptr;
RegOffset += NeedPadding ? 1 : 0;

// Try to use the original type for coercion.
llvm::Type *CoerceTy = CB.isUsableType(StrTy) ? StrTy : CB.getType();

// We use a pair of i64 for 9-16 byte aggregate with 8 byte alignment.
// For 9-16 byte aggregates with 16 byte alignment, we use i128.
llvm::Type *WideTy = llvm::Type::getIntNTy(getVMContext(), 128);
bool UseI128 = (Size > 64) && (Size <= 128) && (Alignment == 128);

if (CB.InReg)
return ABIArgInfo::getDirectInReg(UseI128 ? WideTy : CoerceTy);
return ABIArgInfo::getDirect(UseI128 ? WideTy : CoerceTy);
ABIArgInfo AAI = ABIArgInfo::getDirect(CoerceTy, 0, Padding);
AAI.setInReg(CB.InReg);
return AAI;
}

RValue SparcV9ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
QualType Ty, AggValueSlot Slot) const {
ABIArgInfo AI = classifyType(Ty, 16 * 8);
unsigned ArgOffset = 0;
ABIArgInfo AI = classifyType(Ty, 16 * 8, ArgOffset);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to align the address of the va_arg pointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, though pointers are 64-bit wide and we're already allocating argument registers in 64-bit chunks anyway so it should be aligned properly already.

If what you mean is the contents of the va_list itself, then yes it needs to be aligned too, but I think that's the responsibility of the stack allocator instead of here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The part I'm questioning is, do we need to skip over padding for arguments with 128-bit alignment?

The list as a whole should have 128-bit alignment, sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, yeah, we do need to put alignment paddings too.

llvm::Type *ArgTy = CGT.ConvertType(Ty);
if (AI.canHaveCoerceToType() && !AI.getCoerceToType())
AI.setCoerceToType(ArgTy);
Expand Down Expand Up @@ -346,9 +368,12 @@ RValue SparcV9ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
}

void SparcV9ABIInfo::computeInfo(CGFunctionInfo &FI) const {
FI.getReturnInfo() = classifyType(FI.getReturnType(), 32 * 8);
unsigned RetOffset = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do ArgOffset and RetOffset have to be connected somehow? Returns can be lowered to an argument, which takes a register. Not sure if that register is an argument register in the SPARC calling convention; if it isn't, this is fine, I guess.

Are the padding rules the same on the stack as they are in registers, for functions that take many arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof yeah, if a return is converted to a pointer argument it'll take an argument register...
So in that specific case the argument and return offsets should be connected, yes.

As for the latter, stack arguments have the same padding (or rather, alignment) requirements as they do in registers; there has to be a 1:1 correspondence between registers and stack memory locations.

FI.getReturnInfo() = classifyType(FI.getReturnType(), 32 * 8, RetOffset);

unsigned ArgOffset = 0;
for (auto &I : FI.arguments())
I.info = classifyType(I.type, 16 * 8);
I.info = classifyType(I.type, 16 * 8, ArgOffset);
}

namespace {
Expand Down
24 changes: 19 additions & 5 deletions clang/test/CodeGen/sparcv9-abi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,35 @@ long double f_ld(long double x) { return x; }
struct empty {};
struct emptyarr { struct empty a[10]; };

// 16-byte structs with 16-byte alignment gets passed as if i128.
struct align16 { _Alignas(16) int x; };
// In 16-byte structs, 16-byte aligned members are expanded
// to their corresponding i128/f128 types.
struct align16_int { _Alignas(16) int x; };
struct align16_mixed { _Alignas(16) int x; double y; };
struct align16_longdouble { long double x; };

// CHECK-LABEL: define{{.*}} i64 @f_empty(i64 %x.coerce)
struct empty f_empty(struct empty x) { return x; }

// CHECK-LABEL: define{{.*}} i64 @f_emptyarr(i64 %x.coerce)
struct empty f_emptyarr(struct emptyarr x) { return x.a[0]; }

// CHECK-LABEL: define{{.*}} void @f_aligncaller(i128 %a.coerce)
void f_aligncallee(int pad, struct align16 a);
void f_aligncaller(struct align16 a) {
// CHECK-LABEL: define{{.*}} void @f_aligncaller(i64 %a.coerce0, i64 %a.coerce1)
// CHECK-LABEL: declare{{.*}} void @f_aligncallee(i32 noundef signext, i64, i64, i64)
void f_aligncallee(int pad, struct align16_int a);
void f_aligncaller(struct align16_int a) {
f_aligncallee(0, a);
}

// CHECK-LABEL: define{{.*}} double @f_mixed_aligned(i64 noundef %a, i64 %0, i64 %b.coerce0, double %b.coerce1)
double f_mixed_aligned(long a, struct align16_mixed b) {
return b.y;
}

// CHECK-LABEL: define{{.*}} fp128 @f_longdouble(i64 noundef %a, i64 %0, fp128 %b.coerce)
long double f_longdouble(long a, struct align16_longdouble b) {
return b.x;
}

// CHECK-LABEL: define{{.*}} i64 @f_emptyvar(i32 noundef zeroext %count, ...)
long f_emptyvar(unsigned count, ...) {
long ret;
Expand Down
Loading