Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Add 8548 CPU definition and attributes
ClosedPublic

Authored by jhibbits on Sep 19 2019, 8:20 PM.

Details

Summary

8548 CPU is GCC's name for the e500v2, so accept this in clang. The
e500v2 doesn't support lwsync, so define NO_LWSYNC for this as well,
as GCC does.

Event Timeline

jhibbits created this revision.Sep 19 2019, 8:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 19 2019, 8:20 PM

I made 8548 an alias in clang to e500, because e500 is recognized in llvm as a CPU, so gets us the feature list and, more importantly, the instruction scheduler.

@jhibbits Should not the test in test/Preprocessor/init.c also ensure that __SPE__ and __NO_FPRS__ macros are defined with CPU 8548? The rest looks good to me, thank you.

@vit9696 The only thing GCC defines for mcpu=8548 is NO_LWSYNC, the SPE-specific #defines are from -mspe. That said, since I explicitly do enable SPE for e500 CPU, I guess it makes sense to add it to the test.

Yes, that's the point, since 8548 is an alias.

A side note regarding SPE support. I am currently upgrading to LLVM 9.0 and I discovered that this patch was not committed anyhow at all:
https://reviews.llvm.org/D54583#1444288

A side note regarding SPE support. I am currently upgrading to LLVM 9.0 and I discovered that this patch was not committed anyhow at all:
https://reviews.llvm.org/D54583#1444288

Yeah, I noticed that after I committed it. Problem is, to submit that for review, I need a valid reduced test case., and my laziness/busy-ness has prevented me from creating a reduced test case in LLVM language.

jhibbits updated this revision to Diff 226529.Oct 25 2019, 8:46 PM

Make 8548 actually denote e500 LLVM target, add SPE checks to 8548 preprocessor test.

I noticed a typo in the file, but the rest looks good now.

clang/lib/Driver/ToolChains/Arch/PPC.cpp
61

That looks like a typo to me. There is one more 8548 case above.

jhibbits marked an inline comment as done.Oct 27 2019, 12:37 PM
jhibbits added inline comments.
clang/lib/Driver/ToolChains/Arch/PPC.cpp
61

You're right, I missed that I added it above already.

jhibbits updated this revision to Diff 226583.Oct 27 2019, 12:38 PM

Don't double-check 8548 CPU for setting LLVM CPU type.

vit9696 accepted this revision.Oct 28 2019, 2:06 AM

Thanks.

This revision is now accepted and ready to land.Oct 28 2019, 2:06 AM
jhibbits updated this revision to Diff 228589.Nov 9 2019, 11:51 AM

Add clang-translation tests for e500 and 8548 CPU definitions.

@vit9696 mind reviewing again with the added tests?

jhibbits requested review of this revision.Nov 11 2019, 2:14 PM
vit9696 accepted this revision.Nov 12 2019, 3:52 AM

Actually thanks for adding C checks too. The new revision also looks good to me.

This revision is now accepted and ready to land.Nov 12 2019, 3:52 AM
This revision was automatically updated to reflect the committed changes.