Page MenuHomePhabricator

Introduce support for PowerPC devices with an Embedded Floating-point APU version 2 (efpu2)
ClosedPublic

Authored by kiausch on Dec 9 2020, 4:46 AM.

Details

Summary

PowerPC cores like e200z759n3 [1] using an efpu2 only support single precision hardware floating point instructions.
The single precision instructions efs* and evfs* are identical to the spe float instructions while efd* and evfd* instructions trigger a not implemented exception.

This patch introduces a new command line option -mefpu2 which leads to single-hardware / double-software code generation.

The regression tests in spe.ll were updated so that the tests are executed twice - with and without -mefpu2 option.

[1] Core reference: https://www.nxp.com/files-static/32bit/doc/ref_manual/e200z759CRM.pdf

Diff Detail

Event Timeline

kiausch created this revision.Dec 9 2020, 4:46 AM
kiausch requested review of this revision.Dec 9 2020, 4:46 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 9 2020, 4:46 AM
kiausch updated this revision to Diff 310853.Dec 10 2020, 5:00 AM

fixed format errors

steven.zhang added a reviewer: Restricted Project.Jan 4 2021, 2:46 AM

Some code style comments.

clang/include/clang/Driver/Options.td
3043

Maybe, clang/docs/ClangCommandLineReference.rst need to be updated also ?

llvm/lib/Target/PowerPC/PPCISelLowering.h
707 ↗(On Diff #310853)

I think we don't need such wrap but just call Subtarget.hasEFPU2() directly.

llvm/lib/Target/PowerPC/PPCSubtarget.h
103

Miss to initialize it ?

nemanjai accepted this revision.Jan 4 2021, 3:26 AM

I don't really have any objections to this, but if there is a similar options to GCC, I would hope that they are the same. Please add a comment in the commit message to that end.
Other than that, LGTM.

llvm/test/CodeGen/PowerPC/efpu2.ll
6 ↗(On Diff #310853)

It might make sense to just add a RUN line in that test case rather than stating in a comment that they are identical (i.e. they may not be identical if the original one changes down the road).

This revision is now accepted and ready to land.Jan 4 2021, 3:26 AM

Regarding similar GCC options:

AFAIK GCC has had the spe options -msingle-float and -mdouble-float until spe support was dropped after version 8.3.
These options would kind of match here, but they are already used as MIPS subtarget features in LLVM and I was not able to find out if and how it is possible to use identical target features for multiple targets and if this is a good idea at all? If somebody could point out how this can be done I'm happy to change the options...

kiausch updated this revision to Diff 314566.Jan 5 2021, 4:34 AM
kiausch edited the summary of this revision. (Show Details)

implemented review suggestions:

  • updated clang/docs/ClangCommandLineReference.rst
  • removed unneccessary HasEFPU2() wrapper function
  • initialize HasEFPU member
  • merged tests into spe.ll to remove identical parts
kiausch marked 4 inline comments as done.Jan 5 2021, 5:37 AM
kiausch added inline comments.
llvm/test/CodeGen/PowerPC/efpu2.ll
6 ↗(On Diff #310853)

Merged the tests into spe.ll which is now executed multiple times using additional RUN lines.
Using split-file the file is separeted into a single-precision part which is supposed to generate identical code to the spe implementation and a double-precision part which is checked to generate different results for the efpu2 option.
Is that kind of what you had in mind?

nemanjai added a comment.EditedJan 5 2021, 6:47 AM

Regarding similar GCC options:

AFAIK GCC has had the spe options -msingle-float and -mdouble-float until spe support was dropped after version 8.3.
These options would kind of match here, but they are already used as MIPS subtarget features in LLVM and I was not able to find out if and how it is possible to use identical target features for multiple targets and if this is a good idea at all? If somebody could point out how this can be done I'm happy to change the options...

If GCC still supported the options, it would make sense to go through the effort of making the options work for PPC as well so that the two compilers are compatible. However, since GCC no longer has SPE support, the option interface is mostly up to you.

clang/include/clang/Driver/Options.td
3043

You might want to define what the expected behaviour if -mefpu2 is specified without specifying -mspe. With the current implementation, it will simply be ignored. But you might want to make it so that specifying -mefpu2 automatically causes -mspe to be turned on (and -mno-spe to turn off both SPE and EFPU2). This can be accomplished in PPCTargetInfo::setFeatureEnabled() in clang/lib/Basic/Targets/PPC.cpp.

In any case, you should add a test to clang/test/Driver/ppc-features.cpp.

kiausch updated this revision to Diff 315119.Jan 7 2021, 7:11 AM
kiausch marked an inline comment as done.
  • enable -mspe if -mefpu2 is defined in clang interface
  • added clang feature test
kiausch marked an inline comment as done.Jan 7 2021, 7:12 AM

Could somebody with write access please commit this patch if there are no further objections? Thanks!

This revision was automatically updated to reflect the committed changes.