[llvm] c821ef4 - [LangRef] Make lifetime intrinsic's semantics consistent with StackColoring's comment

[Philip Reames via llvm-commits]

I see a couple of problems with the drafting on this that I think 
warrant tweaking.

1) The original wording for lifetime.start and lifetime.end applied 
equally to all memory objects, not just stack objects.  I think it's a 
mistake to weaken them so much for non-stack objects.  Glancing through 
the optimizer, I can't find a case where this actively causes a problem, 
but the lack of symmetry in the definition seems undesirable.

2) In the wording for lifetime.start, "If the offset is non-zero" is 
very unclear as to meaning.  I think you mean that the pointer passed to 
the object is not the base of the memory object.  At a minimum, this 
should be clarified.  I think we can also make this case immediate UB, 
but am not going to argue strongly for it.

3) The meaning of "dead" changed in this revision in a problematic way.  
The original wording said that loads from dead objects were undef.  The 
new wording says that accessing a dead object is immediate UB.  From the 
commit comment, this doesn't appear to have been an intentional change.  
I ask that this be reversed and reviewed separately if actually 
desired.  If nothing else, there's a backwards compatibility concern here.

Philip

On 3/3/21 4:58 PM, Juneyoung Lee via llvm-commits wrote:
> Author: Juneyoung Lee
> Date: 2021-03-04T09:58:06+09:00
> New Revision: c821ef451373849c403686042466d30a7bf8138a
>
> URL: https://github.com/llvm/llvm-project/commit/c821ef451373849c403686042466d30a7bf8138a
> DIFF: https://github.com/llvm/llvm-project/commit/c821ef451373849c403686042466d30a7bf8138a.diff
>
> LOG: [LangRef] Make lifetime intrinsic's semantics consistent with StackColoring's comment
>
> This patch is an update to LangRef by describing lifetime intrinsics' behavior
> by following the description of MIR's LIFETIME_START/LIFETIME_END markers
> at StackColoring.cpp (https://github.com/llvm/llvm-project/blob/eb44682d671d66e422b02595a636050582a4d84a/llvm/lib/CodeGen/StackColoring.cpp#L163) and the discussion in llvm-dev.
>
> In order to explicitly define the meaning of an object lifetime, I added 'Object Lifetime' subsection.
>
> Reviewed By: nlopes
>
> Differential Revision: https://reviews.llvm.org/D94002
>
> Added:
>      
>
> Modified:
>      llvm/docs/LangRef.rst
>
> Removed:
>      
>
>
> ################################################################################
> diff  --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
> index cc2fcef30831..6866cc362f57 100644
> --- a/llvm/docs/LangRef.rst
> +++ b/llvm/docs/LangRef.rst
> @@ -2544,6 +2544,33 @@ This information is passed along to the backend so that it generates
>   code for the proper architecture. It's possible to override this on the
>   command line with the ``-mtriple`` command line option.
>   
> +.. _objectlifetime:
> +
> +Object Lifetime
> +----------------------
> +
> +A memory object, or simply object, is a region of a memory space that is
> +reserved by a memory allocation such as :ref:`alloca <i_alloca>`, heap
> +allocation calls, and global variable definitions.
> +Once it is allocated, the bytes stored in the region can only be read or written
> +through a pointer that is :ref:`based on <_pointeraliasing>` the allocation
> +value.
> +If a pointer that is not based on the object tries to read or write to the
> +object, it is undefined behavior.
> +
> +A lifetime of a memory object is a property that decides its accessibility.
> +Unless stated otherwise, a memory object is alive since its allocation, and
> +dead after its deallocation.
> +It is undefined behavior to access a memory object that isn't alive, but
> +operations that don't dereference it such as
> +:ref:`getelementptr <i_getelementptr>`, :ref:`ptrtoint <i_ptrtoint>` and
> +:ref:`icmp <i_icmp>` return a valid result.
> +This explains code motion of these instructions across operations that
> +impact the object's lifetime.
> +A stack object's lifetime can be explicitly specified using
> +:ref:`llvm.lifetime.start <_int_lifestart>` and
> +:ref:`llvm.lifetime.end <_int_lifeend>` intrinsic function calls.
> +
>   .. _pointeraliasing:
>   
>   Pointer Aliasing Rules
> @@ -17808,8 +17835,9 @@ Other targets may support this intrinsic
> diff erently, for example, by lowering i
>   Memory Use Markers
>   ------------------
>   
> -This class of intrinsics provides information about the lifetime of
> -memory objects and ranges where variables are immutable.
> +This class of intrinsics provides information about the
> +:ref:`lifetime of memory objects <_objectlifetime>` and ranges where variables
> +are immutable.
>   
>   .. _int_lifestart:
>   
> @@ -17826,8 +17854,8 @@ Syntax:
>   Overview:
>   """""""""
>   
> -The '``llvm.lifetime.start``' intrinsic specifies the start of a memory
> -object's lifetime.
> +The '``llvm.lifetime.start``' intrinsic specifies the start of
> +:ref:`a memory object's lifetime <_objectlifetime>`.
>   
>   Arguments:
>   """"""""""
> @@ -17839,10 +17867,23 @@ to the object.
>   Semantics:
>   """"""""""
>   
> -This intrinsic indicates that before this point in the code, the value
> -of the memory pointed to by ``ptr`` is dead. This means that it is known
> -to never be used and has an undefined value. A load from the pointer
> -that precedes this intrinsic can be replaced with ``'undef'``.
> +If ``ptr`` is a stack-allocated object and its offset is zero, the object is
> +initially marked as dead.
> +After '``llvm.lifetime.start``', the stack object that ``ptr`` points is marked
> +as alive and has an uninitialized value.
> +The stack object is marked as dead when either
> +:ref:`llvm.lifetime.end <int_lifeend>` to the alloca is executed or the
> +function returns.
> +
> +After :ref:`llvm.lifetime.end <int_lifeend>` is called,
> +'``llvm.lifetime.start``' on the stack object can be called again.
> +The second '``llvm.lifetime.start``' call marks the object as alive, but it
> +does not change the address of the object.
> +
> +If ``ptr`` is a non-stack-allocated object, its offset is non-zero or it is
> +a stack object that is already alive, it simply fills all bytes of the object
> +with ``poison``.
> +
>   
>   .. _int_lifeend:
>   
> @@ -17859,8 +17900,8 @@ Syntax:
>   Overview:
>   """""""""
>   
> -The '``llvm.lifetime.end``' intrinsic specifies the end of a memory
> -object's lifetime.
> +The '``llvm.lifetime.end``' intrinsic specifies the end of
> +:ref:`a memory object's lifetime <_objectlifetime>`.
>   
>   Arguments:
>   """"""""""
> @@ -17872,10 +17913,13 @@ to the object.
>   Semantics:
>   """"""""""
>   
> -This intrinsic indicates that after this point in the code, the value of
> -the memory pointed to by ``ptr`` is dead. This means that it is known to
> -never be used and has an undefined value. Any stores into the memory
> -object following this intrinsic may be removed as dead.
> +If ``ptr`` is a stack-allocated object and its offset is zero, the object is
> +dead.
> +Calling ``llvm.lifetime.end`` on an already dead alloca is no-op.
> +
> +If ``ptr`` is a non-stack-allocated object or its offset is non-zero,
> +it is equivalent to simply filling all bytes of the object with ``poison``.
> +
>   
>   '``llvm.invariant.start``' Intrinsic
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
>
>          
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits