Page MenuHomePhabricator

[Draft][LangRef] Document `elementtype` attribute requirement on atomic memory intrinsics.
Needs ReviewPublic

Authored by dantrushin on Jun 15 2022, 11:08 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

LLVM officially supports Garbage Collection environments, including those
requiring read and/or write barriers for object pointer loads/stores.
Such environments must be able to detect type of accessed object.
With opaque pointers the only possible way for memory intrinsics is
elementtype argument attribute. Document this requirement in description
of atomic memory intrinsics.

Diff Detail

Event Timeline

dantrushin created this revision.Jun 15 2022, 11:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2022, 11:08 AM
Herald added a subscriber: jdoerfert. · View Herald Transcript
dantrushin requested review of this revision.Jun 15 2022, 11:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2022, 11:08 AM

Not familiar with GC requirements, but this at least sounds plausible to me (maybe @reames can confirm?) However, I think that if we go down this line, we should probably go all in: Drop the element_size argument from the intrinsics and make elementtype required by the IR verifier, and adjust lowering to perform copies in the correct type.

dantrushin retitled this revision from [LangRef] Document `elementtype` attribute requirement on atomic memory intrinsics. to [Draft][LangRef] Document `elementtype` attribute requirement on atomic memory intrinsics..Jun 15 2022, 12:24 PM

I don't understand the motivation here. Clearly, such an environment could tag the operations this way, and probably should. However, why does that need to be in the LangRef as a requirement? Is the idea to disallow some particular transform in tree that can't meet this?

The optimizer can insert new atomic memcpy/memmove calls, e.g loop-idiom-recognize would do that. Currently, when loop-idiom-recognize inserts such a call it doesn't tag it with the element type, which makes it an incorrect transform for us.

If we document the requirement in the langref we can a) add a verification rule to catch these situations, b) fix such transforms upstream. I think we can restrict this requirement to functions that have a GC strategy specified.

Idea is being able to push something like these though upstream:
https://reviews.llvm.org/D125690
https://reviews.llvm.org/D127892
From our downstream point of view they look reasonable, but we've encountered upstream resistance on first (so most likely will see it on second as well if we do it unconditionally)