This is an archive of the discontinued LLVM Phabricator instance.

[ARM][Clang] Removing lowering of half-precision FP arguments and returns from Clang's CodeGen
ClosedPublic

Authored by pratlucas on Jun 9 2020, 2:23 AM.

Details

Summary

On the process of moving the argument lowering handling for
half-precision floating point arguments and returns to the backend, this
patch removes the code that was responsible for handling the coercion of
those arguments in Clang's Codegen.

Diff Detail

Event Timeline

pratlucas created this revision.Jun 9 2020, 2:23 AM
ostannard accepted this revision.Jun 10 2020, 2:29 AM

LGTM, thanks for working on this patch series, this looks much clearer than doing it in clang.

This revision is now accepted and ready to land.Jun 10 2020, 2:29 AM
stuij added a subscriber: stuij.Jun 15 2020, 5:44 AM

I would have expected changes for neon as well.

Can't we now also get rid of the HasLegalHalfType argument to GetNeonType in CGBuiltin.(h|cpp). And also similar code in TargetInfo.cpp (ARMABIInfo::classifyReturnType, ARMABIInfo::isIllegalVectorType)?

Hi @stuij,

The changes to the backend only handle the half (f16) type itself, not vectors that have it as their base type.

From what I've checked on the AAPCS, the rules for handling those types are a bit different and they would require their own handling in the backend's calling convention lowering.
I haven't looked into the backend's handling of those types in detail, but I believe a similar approach to the one taken for f16 would be possible for the vector types as well.

Hi @stuij,

The changes to the backend only handle the half (f16) type itself, not vectors that have it as their base type.

From what I've checked on the AAPCS, the rules for handling those types are a bit different and they would require their own handling in the backend's calling convention lowering.
I haven't looked into the backend's handling of those types in detail, but I believe a similar approach to the one taken for f16 would be possible for the vector types as well.

Fair enough. Thanks.

This revision was automatically updated to reflect the committed changes.
clang/test/CodeGen/arm-mve-intrinsics/vsubq.c