Page MenuHomePhabricator

[PowerPC][AIX] Add support for varargs for complex types on AIX
ClosedPublic

Authored by ZarkoCA on Jul 20 2021, 1:12 PM.

Details

Summary

Remove the previous error and add support for special handling of small
complex types as in PPC64 ELF ABI. As in, generate code to load from
varargs location and pack it in a temp variable, then return a pointer to
the struct.

Diff Detail

Event Timeline

ZarkoCA created this revision.Jul 20 2021, 1:12 PM
ZarkoCA requested review of this revision.Jul 20 2021, 1:12 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 20 2021, 1:12 PM
hubert.reinterpretcast added inline comments.
clang/lib/CodeGen/TargetInfo.cpp
4670–4676

The 32-bit tests suggest that the "8 bytes"/"doublewords" in the above are not always so.

ZarkoCA updated this revision to Diff 360535.Jul 21 2021, 11:24 AM
  • Fix comment to also describe what happens in 32bit mode
clang/lib/CodeGen/TargetInfo.cpp
4670–4676

Yes, I missed updating that part of the comment previously, thank you.

ZarkoCA updated this revision to Diff 369112.Fri, Aug 27, 8:53 AM

Rebase to ToT

I suggest we separate the clang change and testing into a standalone patch, and the llvm backend tests into a standalone patch which we can commit separately.

clang/lib/CodeGen/TargetInfo.cpp
4678

Minor nit: the code for PPC64 and this is almost identical, I think it should be factored into a separate helper function.

clang/test/CodeGen/aix32-complex-varargs.c
3

The code-gen for int and float won't change with this patch, lets pre-commit this test without the _Complex short and _Complex char portions now as an NFC patch.

ZarkoCA planned changes to this revision.Tue, Sep 14, 11:46 AM
ZarkoCA added inline comments.Tue, Sep 14, 1:59 PM
clang/test/CodeGen/aix32-complex-varargs.c
3

I can't precommit this test unless I also remove the fatal error here:

if (Ty->isAnyComplexType())
   llvm::report_fatal_error("complex type is not supported on AIX yet");

But I believe that I can commit the llc tests and I'll go ahead and do that.

ZarkoCA added inline comments.Tue, Sep 14, 2:01 PM
clang/test/CodeGen/aix32-complex-varargs.c
3

Sorry, not commit the llc tests but separate them in their own patch.

ZarkoCA updated this revision to Diff 372736.Wed, Sep 15, 9:56 AM
  • Removed llc tests
  • Added a helper function to combine the PPC64_SVR4ABI and AIXABI treatment of complex types less than register size

I suggest we separate the clang change and testing into a standalone patch, and the llvm backend tests into a standalone patch which we can commit separately.

LLVM tests added in https://reviews.llvm.org/D109838

sfertile accepted this revision as: sfertile.Wed, Sep 15, 1:06 PM

LGTM.

clang/test/CodeGen/aix32-complex-varargs.c
3

Sorry, I missed that. I was thinking since the word-size element types didn't change we should recommit heir tests, but since it was disabled with a fatal error that doesn't make sense.

This revision is now accepted and ready to land.Wed, Sep 15, 1:06 PM