This is an archive of the discontinued LLVM Phabricator instance.

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.

Diff Detail

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 ↗(On Diff #226529)

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 ↗(On Diff #226529)

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.