[LangRef] Limit read/writeonly attrs to memory visible to caller

[fhahn]

Summary

This patch unifies the wording used for both readonly and writeonly
attributes. Both descriptions more specifically refer to memory visible
to caller functions, like in the first paragraph of the readonly
definition.

The motivation for the clarification is D123473.

[fhahn]

Use caller functions for the last paragraphs of both writeonly and readonly.

[nlopes]

Not sure it's sufficient to restrict to the callers. Essentially we want to assert that these functions don't have state, so any read/writes they do is to locally allocated memory. No other function in the program can access that memory.

[efriedma]

"readonly" really has to encompass memory visible to the whole unit of optimization (the current translation unit, plus any other translation units involved in LTO) to justify the way we reason about it. For example, suppose we have a function that's logically doesn't have any state, but it contains a mutable cache of results. Then suppose we inline one call to it: now the "readonly" function touches state that matters to the caller.

"caller functions" is also a bit imprecise if callbacks are involved.

It might be possible to extend LLVM IR to define some memory as "internal" to some particular set of functions, and opaque to outside functions, but that currently doesn't exist.

[efriedma]

Oh, just took a look at D123473. I see what you're trying to do... yes, we should allow writing to memory that's allocated/deallocated within a function.

I think the only way to make the requirement here clear is to spell out the distinction between memory allocations that continue to be accessible after the function returns, vs. temporary memory allocations in the implementation. Maybe something like the following:

"We define 'local memory' to be memory that is allocated after entry to a function, deallocated before control flow returns to the caller. If a readonly function writes to memory that is not local memory, or has other side effects, the behavior is undefined."

(I'm not sure if we need to explicitly restrict accesses to local memory from other threads? I guess other threads don't have any way to construct a pointer to local memory, so that's probably okay.)

[fhahn]

In D124124#3463817, @efriedma wrote:

Oh, just took a look at D123473. I see what you're trying to do... yes, we should allow writing to memory that's allocated/deallocated within a function.

I think the only way to make the requirement here clear is to spell out the distinction between memory allocations that continue to be accessible after the function returns, vs. temporary memory allocations in the implementation. Maybe something like the following:

"We define 'local memory' to be memory that is allocated after entry to a function, deallocated before control flow returns to the caller. If a readonly function writes to memory that is not local memory, or has other side effects, the behavior is undefined."

(I'm not sure if we need to explicitly restrict accesses to local memory from other threads? I guess other threads don't have any way to construct a pointer to local memory, so that's probably okay.)

Thanks @nlopes & @efriedma! I agree that caller functions is not sufficient in general. I updated the wording to use visible outside the function . This would also disallow accessing memory that has been allocated outside the function and gets deallocated in the function.

Eli's wording for local memory looks good to me too, but if we use it for multiple attributes it would be good to define it once and refer back to it, to avoid duplication. I couldn't find a good place to move such a definition, hence I only updated the wording to visible outside the function for now. Happy to move things around if needed.

In practice I think local memory will likely not include dynamically allocated memory, even if it is deallocated before exiting the function, as most allocators will modify state visible outside the function.

[nlopes]

LGTM, I think it reads well.

[efriedma]

"outside the function" doesn't really make it clear that you're talking about a dynamic invocation of the function, as opposed to instructions inside the function's body. But maybe it's close enough.

[fhahn]

In D124124#3472154, @efriedma wrote:

"outside the function" doesn't really make it clear that you're talking about a dynamic invocation of the function, as opposed to instructions inside the function's body. But maybe it's close enough.

Hmm, maybe it could be even clearer. Do you have any suggestions? I guess we could try to spell out a definition of function-local memory somewhere and refer back to it, but I am not sure where the best place would be;

[efriedma]

It might make sense to have a separate LangRef section for "memory-related function attributes", to describe all the assumptions we're making in one place.

llvm/docs/LangRef.rst

``readonly``
     cannot unwind exceptions by calling the ``C++`` exception throwing methods
     (since they write to memory), there may be non-``C++`` mechanisms that throw
     exceptions without writing to LLVM visible memory.
 
     On an argument, this attribute indicates that the function does not write
     through this pointer argument, even though it may write to the memory that
     the pointer points to.
 
-    If a readonly function writes memory visible to the program, or
+    If a readonly function writes memory visible to a caller function, or
     has other side-effects, the behavior is undefined. If a function writes to
     a readonly pointer argument, the behavior is undefined.
 ``"stack-probe-size"``
     This attribute controls the behavior of stack probes: either
     the ``"probe-stack"`` attribute, or ABI-required stack probes, if any.
     It defines the size of the guard region. It ensures that if the function
     may use more stack space than the size of the guard region, stack probing
     sequence will be emitted. It takes one required integer value, which
     is 4096 by default.
 
     If a function that has a ``"stack-probe-size"`` attribute is inlined into
     a function with another ``"stack-probe-size"`` attribute, the resulting
     function has the ``"stack-probe-size"`` attribute that has the lower
     numeric value. If a function that has a ``"stack-probe-size"`` attribute is
     inlined into a function that has no ``"stack-probe-size"`` attribute
     at all, the resulting function has the ``"stack-probe-size"`` attribute
     of the callee.
 ``"no-stack-arg-probe"``
     This attribute disables ABI-required stack probes, if any.
 ``writeonly``
     On a function, this attribute indicates that the function may write to but
-    does not read from memory.
+    does not read from memory visible to caller functions.
 
     On an argument, this attribute indicates that the function may write to but
     does not read through this pointer argument (even though it may read from
     the memory that the pointer points to).
 
-    If a writeonly function reads memory visible to the program, or
+    If a writeonly function reads memory visible to caller functions, or
     has other side-effects, the behavior is undefined. If a function reads
     from a writeonly pointer argument, the behavior is undefined.
 ``argmemonly``
     This attribute indicates that the only memory accesses inside function are
     loads and stores from objects pointed to by its pointer-typed arguments,
     with arbitrary offsets. Or in other words, all memory operations in the
     function can refer to memory only using pointers based on its function
     arguments.