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

Sat May 9 16:29:26 PDT 2020

jdoerfert added a comment.

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

> > Do you object to say that the call site argument and the argument point to distinct memory locations or something else?
>
> Like I said, my issue is with the "Attributes on the call site argument and function argument are associated with the original and copied memory respectively".  I assume this means "attributes other than byval".

Yes. `byval` applies to both.

> If I'm understanding this correctly, this means it isn't legal to copy attributes from the caller to the callee.

Correct. It is not legal to do that now either, see below, but we just don't say so. TBH, I have not heard a proposal in which it would be legal but the copy would still happen implicitly (somewhere).

> If an argument is marked "readnone byval" on a function, it's illegal to copy that "readnone" to the callsite, because the readnone would then be associated with the original memory, not the copied memory.

Right. That is what I think needs to be the semantics.

> Or, a more silly example, say you had "byval returned" on the called function, and that got copied to the callsite: that clearly can't mean the original pointer is returned by the function.

I think `byval returned` example shows nicely that we cannot copy the attributes, right?

@efriedma I'm a bit confused. Could you propose some wording so I get a feeling where you want to go?

In D79636#2028363 <https://reviews.llvm.org/D79636#2028363>, @aqjune wrote:

> I have a minor question:
>
> > a call of a readnone function with a byval argument is not classified as readnone (which it is today: https://godbolt.org/z/dDfQ5r)
>
> %0 at caller has readnone attribute - is it related with the propagation of readnone attribute from %0 of empty function to the caller?
>  Some comments above seems to be related with this question, but I rather wonder about the validity of the propagation of readnone in this example.

The propagation in this example is *not* valid. This patch makes this clear (I hope).

> Actually I wonder whether things will become clearer if an alloca-and-copy (or something that is equivalent with this) is explicitly used to show the behavior of pass-as-value rather than byval implicitly encoding the behavior; my impression is that byval is different from other attributes like readnone or nonnull, because it isn't the result of value analysis. This will be a lot of work though...

You are not wrong. Making it explicit would actually help us. I am in favor. Nonetheless, we currently seem to have no clear semantics on what `byval` means and how it interacts with other attributes. Clang strips `__attribute__((pure))` if `byval` arguments are present but `functionattrs` will just add it again. More generally, this does currently not work:

  for (Instruction *I : instructions(Fn))
    MayReadOrWrite |= I->mayReadOrWriteMemory();

if a call takes a `byval` argument.

Long story short, I would prefer this change to make the current behavior consistent and then a transition away from `byval` to some explicit copy model.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79636