Page MenuHomePhabricator

[clang][aarch64] Fix ILP32 ABI for arm_sve_vector_bits
ClosedPublic

Authored by c-rhodes on Sep 9 2020, 3:22 AM.

Details

Summary

The element types of scalable vectors are defined in terms of stdint
types in the ACLE. This patch fixes the mapping to builtin types for the
ILP32 ABI when creating VLS types with the arm_sve_vector_bits, where
the mapping is as follows:

int32_t -> LongTy
int64_t -> LongLongTy
uint32_t -> UnsignedLongTy
uint64_t -> UnsignedLongLongTy

This is implemented by leveraging getBuiltinVectorTypeInfo which is
target agnostic since it calls ASTContext::getIntTypeForBitwidth for
integer types. The element type for svfloat16_t is changed from
Float16Ty to HalfTy when creating VLS types since this is what is used
elsewhere.

For more information, see:

https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#types-varying-by-data-model
https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#appendix-support-for-scalable-vectors

Diff Detail

Event Timeline

c-rhodes created this revision.Sep 9 2020, 3:22 AM
Herald added a project: Restricted Project. · View Herald Transcript
c-rhodes requested review of this revision.Sep 9 2020, 3:22 AM
sdesmalen added inline comments.Sep 9 2020, 9:54 AM
clang/lib/AST/Type.cpp
2324–2325

Rather than comparing with a specific triple, how about getting the information from TargetInfo, i.e.

case BuiltinType::SveInt32:
  Ctx.getTargetInfo().getLongWidth() == 32 ? Ctx.LongTy : Ctx.IntTy
efriedma added inline comments.Sep 9 2020, 6:02 PM
clang/lib/AST/Type.cpp
2324–2325

The type that actually corresponds to int64_t is TargetInfo::getInt64Type(). Not sure if you need that here.

c-rhodes updated this revision to Diff 290974.Sep 10 2020, 7:50 AM
c-rhodes edited the summary of this revision. (Show Details)
  • Get the element type of scalable vectors from getBuiltinVectorTypeInfo when creating VLS types. This fixes the ABI issue since it calls ASTContext::getIntTypeForBitwidth which gets the correct type for a given target.
  • Element type for svfloat16_t is changed from Float16Ty to HalfTy for VLS types since this is what’s used elsewhere.
c-rhodes added inline comments.Sep 10 2020, 7:55 AM
clang/lib/AST/Type.cpp
2324–2325

I realised ASTContext::getIntTypeForBitwidth handles this and noticed getBuiltinVectorTypeInfo which Sander recently added makes use of it so I've updated it to use that instead for all types except predicates since we still need to represent them as i8.

efriedma accepted this revision.Sep 10 2020, 11:28 AM

LGTM

clang/lib/AST/Type.cpp
2325–2326

For 8-bit types, not sure you're getting anything by using getIntTypeForBitwidth instead of Ctx.UnsignedCharTy; there isn't actually any possible variation. I don't care much either way, though.

This revision is now accepted and ready to land.Sep 10 2020, 11:28 AM
This revision was automatically updated to reflect the committed changes.
c-rhodes added inline comments.Sep 11 2020, 3:14 AM
clang/lib/AST/Type.cpp
2325–2326

For 8-bit types, not sure you're getting anything by using getIntTypeForBitwidth instead of Ctx.UnsignedCharTy; there isn't actually any possible variation. I don't care much either way, though.

Yeah you're right, may as well keep Ctx.UnsignedCharTy, I changed it before pushing. Thanks for reviewing!