[PATCH] D79454: [IR] `byval` arguments cause reads at call sites

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Let me explain why I came to the conclusion this is the right choice, we can go from there:

In my mind, a function without memory accesses should be considered `readnone`. Even if memory
accesses are present, `readnone` is fine *iff* the accessed memory is local (alloca) (or constant,
e.g., via tbaa [this is what I've seen other parts do]).

I always assumed `byval` is a copy on the call edge. It does not happen "in the caller before the call"
or "in the callee at the beginning of the function" but "on the call edge" (similar to phi assignment on a CFG edge).
With that mental model (there is a copy made on the call edge) I went to design the access behavior of `byval` in the Attributor. 
It will mark `byval` call site arguments *always* as `readonly` as they are copied. In the callee, the `byval` argument is refering
to the copy, thus considered "local memory", basically like any `alloca`. Accesses to the `byval` argument are not treated as argument
accesses but as local memory accesses.

Now I realized today that some tests we had for the two ideas above match the wrong thing nowadays. 
We currently do derive `readnone` for `byval` call site arguments. That is in my mind a problem:

  define void @empty(i8* byval %rn) readnone {
    ret void
  }
  
  define void @caller(i8* %arg) {      ; neither @caller nor %arg should be considered readnone
    call void @empty(i8* byval %arg) readnone
    ret void
  }

My local fix for the Attributor was ugly as we filter "interesting" instructions early with `mayReadOrWrite()`. The two changes you
see in the patch are those ugly hacks. I thought if the logic I had in mind is sound `mayReadOrWrite` is the right place to check this.

---

Responses:

> Kill of `byval`/make the copy explicit:

Fine with me, doesn't solve the current problem though.

> Is byval part of the calller/do I change anything:

I don't think so. As far as I know, all attributes on an argument are also at the call site, and vice versa. So the attribute is
on both sides of the call edge and the question is what it means. As mentioned above, I think the copy is neither in the callee nor
in the caller. I would assume my interpretation is not really breaking with your that it happens in the callee, assuming it happens "before anything else".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79454