This is an archive of the discontinued LLVM Phabricator instance.

[Intrinsics] Add `nocallback` to the memset/cpy/move intrinsics
ClosedPublic

Authored by jdoerfert on Jul 11 2022, 2:06 PM.

Details

Diff Detail

Event Timeline

jdoerfert created this revision.Jul 11 2022, 2:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2022, 2:06 PM
Herald added a subscriber: bollu. · View Herald Transcript
jdoerfert requested review of this revision.Jul 11 2022, 2:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2022, 2:06 PM
Herald added a subscriber: wdng. · View Herald Transcript
nikic added a subscriber: nikic.Jul 11 2022, 2:22 PM
nikic added inline comments.
llvm/include/llvm/IR/Intrinsics.td
622

Why can't we use DefaultAttrsIntrinsic here? llvm.memcpy can't synchronize, right?

jdoerfert added inline comments.Jul 11 2022, 2:51 PM
llvm/include/llvm/IR/Intrinsics.td
622

The problem is the volatile flag. I don't think we can/should make them nosync as long as we have that one...

nikic added inline comments.Jul 11 2022, 3:15 PM
llvm/include/llvm/IR/Intrinsics.td
622

Ah, I assumed that nosync only covers atomic accesses and not volatile ones, but apparently volatile is explicitly covered by nosync in LangRef:

Synchronization is considered possible in the presence of atomic accesses that enforce an order, thus not “unordered” and “monotonic”, volatile accesses, as well as convergent function calls.

jdoerfert edited the summary of this revision. (Show Details)Jul 11 2022, 7:52 PM
jdoerfert updated this revision to Diff 443821.Jul 11 2022, 7:53 PM
jdoerfert marked 2 inline comments as done.
jdoerfert edited the summary of this revision. (Show Details)

Update tests

jdoerfert edited the summary of this revision. (Show Details)Jul 11 2022, 7:53 PM
nikic accepted this revision.Jul 12 2022, 12:55 AM

LGTM

This revision is now accepted and ready to land.Jul 12 2022, 12:55 AM
This revision was landed with ongoing or failed builds.Jul 21 2022, 8:53 PM
This revision was automatically updated to reflect the committed changes.