[PATCH] D127383: Don't treat readnone call in presplit coroutine as not access memory

Mon Jul 25 09:41:23 PDT 2022

fhahn added a comment.

The follow-up changes required by this patch (D130155 <https://reviews.llvm.org/D130155>, D130153 <https://reviews.llvm.org/D130153>) seem to highlight some gaps with the new modeling . As a side-effect we now consider intrinsics we know *won't* read the thread id as may-read, even if they are marked readonly, which breaks assumptions in a couple of places. There probably will be more. It seems like we should at least have a way to mark functions we know don't access thread ids, rather than updating code to account for the fact that read none intrinsics now are considered may-read in some circumstances.

I have not been through all the previous discussion, but from the langref changes the new behavior of `doesNotAccessMemory` & co is not very clear to me. The langref change  specifically calls out reading the id of the current thread being not considered reading memory. But `doesNotAccessMemory` seems to consider it accessing memory in Coro-split functions? Shouldn't the LangRef's definition at least clearly call out the interaction with the `presplitcoroutine` attribute?

In general, reasoning about the interaction between various memory attributes is already quite tricky and making their behavior dependent on a different attribute seems to make it even more difficult.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127383/new/

https://reviews.llvm.org/D127383