This is an archive of the discontinued LLVM Phabricator instance.

Document __builtin_nontemporal_load and __builtin_nontemporal_store.
ClosedPublic

Authored by mzolotukhin on Sep 10 2015, 4:52 PM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

mzolotukhin retitled this revision from to Document __builtin_nontemporal_load and __builtin_nontemporal_store..
mzolotukhin updated this object.
mzolotukhin added reviewers: hfinkel, rsmith.
mzolotukhin added a subscriber: cfe-commits.
rsmith added inline comments.Sep 10 2015, 5:10 PM
docs/LanguageExtensions.rst
1802–1807 ↗(On Diff #34511)

This seems to make the feature essentially useless, since you cannot guarantee that the address register is set up sufficiently far before the non-temporal load. Should the compiler not be required to insert the necessary barrier itself in this case?

mzolotukhin added inline comments.Sep 10 2015, 5:57 PM
docs/LanguageExtensions.rst
1802–1807 ↗(On Diff #34511)

Yes, we can require targets to only use corresponding NT instructions when it's safe, and then remove this remark from the documentation. For ARM64 that would mean either not to emit LDNP at all, or conservatively emit barriers before each LDNP (which probably removes all performance benefits of using it) - that is, yes, non-temporal loads would be useless on this target.

But I think we want to keep the builtin for NT-load, as it's a generic feature, not ARM64 specific. It can be used on other targets - e.g. we can use this in x86 stream builtins, and hopefully simplify their current implementation. I don't know about non-temporal operations on other targets, but if there are others, they can use it too right out of the box.

  • Remove paragraph about changing program behavior (since we shouldn't change it anyway).
rsmith accepted this revision.Sep 10 2015, 6:50 PM
rsmith edited edge metadata.

LGTM

docs/LanguageExtensions.rst
1784 ↗(On Diff #34521)

to generate -> generation of

1799 ↗(On Diff #34521)

would -> will

This revision is now accepted and ready to land.Sep 10 2015, 6:50 PM
This revision was automatically updated to reflect the committed changes.

Thanks, committed in r247374 with the requested changes!

Michael