[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D79636#2028479 <https://reviews.llvm.org/D79636#2028479>, @efriedma wrote:

> > @efriedma I'm a bit confused. Could you propose some wording so I get a feeling where you want to go?
>
> Depending on which direction we go, either:
>
> - "Attributes on a function or callsite describe the behavior of the callee excluding the implied copy.  For example, an optimization can infer readnone on a a byval argument if the callee does not access the copied memory."
> - "Attributes on a function or callsite describe the behavior of a call including the implied copy.  For example, an optimization can never infer readnone on a call with a byval argument readnone because the implied copy reads memory.  byval implies nocapture: there isn't any way to retrieve the original address in the callee."
>
>   I don't really care which we choose, we can easily model it either way.  But I think we need to choose one or the other.  Making attributes describe different things on each side of the call is much worse than either of those alternatives; it confuses what it means for a function or callsite to have an attribute.


I think this helped. Thanks. I also understand your concerns now.

Given the choice between the two, I vote for the second:
Pros:

- There is a place the read is attributed to, thus no need for D79454 <https://reviews.llvm.org/D79454>.
- The `byval` -> `nocapture` step at the call site can be useful (done already by the Attributor).
- It also is probably important to have `byval` -> `noalias` in the callee.

Cons:

- You don't get `byval` -> `readonly` at the call site anymore (done by the Attributor).
- Alignment at the call site might be higher than of the copy, breaking with the idea that the call site and callee "properties" match. Though, the attributes can probably be kept in sync if we teach the relevant parts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636