This is an archive of the discontinued LLVM Phabricator instance.

Add support for intrinsics llvm.ppc.dcbfl and llvm.ppc.dcbflp
ClosedPublic

Authored by anil9 on Oct 3 2019, 11:39 AM.

Details

Summary

Added support for the intrinsic llvm.ppc.dcbfl and llvm.ppc.dcbflp. These will be used for emitting cache control instructions dcbfl and dcbflp which are actually mnemonics for using dcbf instruction with different immediate arguments.

dcbfl ra, rb -> dcbf ra, rb, 1
dcbflp, ra, rb -> dcbf ra, rb, 3

Diff Detail

Event Timeline

anil9 created this revision.Oct 3 2019, 11:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 3 2019, 11:39 AM
anil9 edited the summary of this revision. (Show Details)Oct 22 2019, 10:29 AM
anil9 edited the summary of this revision. (Show Details)Oct 22 2019, 10:30 AM
anil9 edited the summary of this revision. (Show Details)Oct 22 2019, 10:48 AM
lei added a subscriber: lei.Oct 22 2019, 11:03 AM

Since these are just mnemonics for using dcbf, why not just add the tests to llvm/test/CodeGen/PowerPC/dcbf.ll instead of creating 2 small test files.

In D68411#1718116, @lei wrote:

Since these are just mnemonics for using dcbf, why not just add the tests to llvm/test/CodeGen/PowerPC/dcbf.ll instead of creating 2 small test files.

These are mnemonics true, but we have separate intrinsics for them as opposed to using one intrinsic with different constants and also the assembly code is different.
I think having multiple files gives more structure and in theory reduces chances of merge conflicts.

In D68411#1718116, @lei wrote:

Since these are just mnemonics for using dcbf, why not just add the tests to llvm/test/CodeGen/PowerPC/dcbf.ll instead of creating 2 small test files.

These are mnemonics true, but we have separate intrinsics for them as opposed to using one intrinsic with different constants and also the assembly code is different.
I think having multiple files gives more structure and in theory reduces chances of merge conflicts.

I second Lei's suggestion to combine the test cases. As lightweight as LIT test cases are, there are already nearly 60k of them and there is a non-zero cost to running additional ones. So if there is no compelling reason to add multiple test cases, we should opt to combine them. And I don't see a compelling reason to separate these.

anil9 updated this revision to Diff 236628.Jan 7 2020, 10:48 AM

Integrated the test cases dcbfl.ll and dcbflp.ll in the previous differential to the previously existing test case dcbf.ll

LGTM other than a minor nit that can be addressed on the commit.

llvm/test/CodeGen/PowerPC/dcbf.ll
32

Perhaps change one of the calls to include an offset to ensure we are correctly materializing the immediate.

nemanjai accepted this revision.Jan 12 2020, 10:17 AM

Forgot to accept.

This revision is now accepted and ready to land.Jan 12 2020, 10:17 AM
anil9 updated this revision to Diff 241538.Jan 30 2020, 12:07 PM

Added an offset to a pointer in one of the tests, for added coverage.

anil9 marked an inline comment as done.Jan 30 2020, 12:07 PM
This revision was automatically updated to reflect the committed changes.