This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Add support for __dcbf builtin
ClosedPublic

Authored by saghir on Mar 26 2019, 1:47 PM.

Details

Summary

This patch is intended to add support for __dcbf builtin.

Purpose:
Data Cache Block Flush: Copies the contents of a modified block from the data cache to main memory and flushes the copy from the data cache.

Prototype:
void __dcbf(const void* addr);

Diff Detail

Repository
rC Clang

Event Timeline

saghir created this revision.Mar 26 2019, 1:47 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 26 2019, 1:47 PM
Herald added a subscriber: jsji. · View Herald Transcript
saghir edited reviewers, added: hfinkel; removed: llvm-commits.Mar 26 2019, 1:51 PM
saghir added a subscriber: llvm-commits.
saghir updated this revision to Diff 192355.Mar 26 2019, 2:17 PM

Updated test in dcbf.ll

stefanp accepted this revision.Mar 28 2019, 11:19 AM

Minor nit that can be fixed when this patch is committed.
LGTM.

llvm/test/CodeGen/PowerPC/dcbf.ll
10 ↗(On Diff #192355)

nit:
I don't think that you need to check the basic block label here. It does not have anything to do with what you are trying to test.

This revision is now accepted and ready to land.Mar 28 2019, 11:19 AM
amyk accepted this revision.Mar 28 2019, 11:26 AM

This looks good to me.

saghir updated this revision to Diff 193957.Apr 5 2019, 1:25 PM

Removed check for basic block in dcbf.ll

It would be great if we could add some documentation for these new added builtins, to make more people to know what we have done. i.e. clang/docs/LanguageExtensions.rst

saghir updated this revision to Diff 194442.Apr 9 2019, 9:50 PM

Added documentation in clang/docs/LanguageExtensions.rst

amyk accepted this revision.Apr 10 2019, 1:52 PM

I think this still looks good to me, with the addition of the documentation.

Looks good, just one minor comments.
It seems that, we have already had the PowerPC section in the rst. It would be better to put them together.

PowerPC Language Extensions
------------------------------

Set the Floating Point Rounding Mode
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
saghir updated this revision to Diff 195094.Apr 14 2019, 10:22 PM

Moved cache builtins section to PowerPC Language extensions in LanguageExtensions.rst

I am ok now for the doc part change. Thank you.

stefanp accepted this revision.Apr 15 2019, 10:59 AM

Looks good to me.
Then only thing I noticed was that in the description in the LanguageExtensions.rst file you use plural to refer to the chache builtins. Of course, you only added one builtin but I know that in the future you plan to add more builtins of the same type so I wouldn't worry about that wording now.

Looks good to me.
Then only thing I noticed was that in the description in the LanguageExtensions.rst file you use plural to refer to the chache builtins. Of course, you only added one builtin but I know that in the future you plan to add more builtins of the same type so I wouldn't worry about that wording now.

That is precisely the reason for using plural since we plan to add support for more cache builtins.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2019, 4:23 PM