This is an archive of the discontinued LLVM Phabricator instance.

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
2603

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
2603

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.
jrtc27 added a subscriber: jrtc27.Jul 28 2021, 7:02 AM
jrtc27 added inline comments.
llvm/test/CodeGen/PowerPC/spe.ll
2–15

This breaks being able to run update_llc_test_checks.py without first running the test itself to generate the files

jrtc27 added inline comments.Jul 28 2021, 7:04 AM
llvm/test/CodeGen/PowerPC/spe.ll
2–15

In fact it breaks it completely, update_llc_test_checks.py doesn't support the %t substitutions, and even if it did it's unclear what that even means; should it update the CHECK lines in the file with the RUN lines or the files being used by FileCheck?

This should have just been manually split.

jrtc27 added inline comments.Jul 28 2021, 7:18 AM
llvm/test/CodeGen/PowerPC/spe.ll
2–15

Most of this is also entirely unnecessary if you use --check-prefixes=CHECK,SPE/EFPU2; update_llc_test_checks.py will merge CHECK lines when possible. The only thing that needs splitting is the hwdouble test, by just putting it in a separate file (or using sed to preprocess the file so SPE sees the test uncommented and EFPU2 sees the test commented out, but that seems unnecessary, though does work). I'll write a patch for the former.

kiausch added inline comments.Jul 28 2021, 7:57 AM
llvm/test/CodeGen/PowerPC/spe.ll
2–15

In the first revison of this patch the tests were split into two separate files for SPE and EFPU2 but this was legitimately criticised for duplicating the single precision tests. This is why I tried to use the split-file feature.
My intention was to

  • run the single.ll part on both, spe and efpu2
  • run the double.ll part on both, spe and efpu2, but with different assertions. When using efpu2 there are no hardware double instructions generated but library calls, and I just wanted to check that in this case the correct calls were genereated. The assertions for these cases were hand-crafted because I was not really able to get the update_llc_test_checks.py to work. The comment above came from the original test file.
  • the hwdouble.ll part needs to be excluded because this uses an assembly instruction which is not available on efpu2 devices.