[llvm] 5737ce2 - [LangRef] Allow non-power-of-two assume operand bundle

[Philip Reames via llvm-commits]

The first part of this makes sense.  The second seems to be going too 
far to me.  Having a non-power-of-two alignment mean the pointer must be 
null seems to overfit to one particular use.

There are other reasonable interpretations of non-power-of-two 
alignments.  For instance, align 48 could mean "16 byte offset from 32 
byte aligned value".  This is in fact the meaning I'm much more familiar 
with.

I think what we should do here is specify the non-power-of-two case not 
as UB, but as "no assumption made".  That allows either interpretation, 
and if we decide we want align variants with the various 
interpretations, we can add them.

Unless you have a very strong argument in favor of specializing the 
align bundle, I ask that you revert that part.

Philip

On 3/23/22 07:53, Nikita Popov via llvm-commits wrote:
> Author: Nikita Popov
> Date: 2022-03-23T15:51:16+01:00
> New Revision: 5737ce259bf5f07976d6f0440e08acd7016a42c8
>
> URL: https://github.com/llvm/llvm-project/commit/5737ce259bf5f07976d6f0440e08acd7016a42c8
> DIFF: https://github.com/llvm/llvm-project/commit/5737ce259bf5f07976d6f0440e08acd7016a42c8.diff
>
> LOG: [LangRef] Allow non-power-of-two assume operand bundle
>
> There has been a lot of confusion on this in the past (see for
> example https://reviews.llvm.org/D110634 and earlier revisions),
> so let's try to get some clarity here. This patch specifies that
> a) specifying a non-constant assumed alignment is explicitly
> allowed and b) an invalid (non-power-of-two) alignment is not UB,
> but rather converts it into an assumption that the pointer is null.
>
> This change is done for two reasons:
> a) Assume operand bundles are specifically used in cases where the
> alignment is not known during frontend codegen (otherwise we'd just
> use an align attribute), so rejecting this case doesn't make sense.
> b) At least for aligned_alloc the C standard specifies that passing
> an invalid alignment results in a null pointer, not undefined
> behavior.
>
> Differential Revision: https://reviews.llvm.org/D119414
>
> Added:
>      
>
> Modified:
>      llvm/docs/LangRef.rst
>
> Removed:
>      
>
>
> ################################################################################
> diff  --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
> index a765d32a42099..f76718c077129 100644
> --- a/llvm/docs/LangRef.rst
> +++ b/llvm/docs/LangRef.rst
> @@ -2428,9 +2428,6 @@ An assume operand bundle has the form:
>   
>   If there are no arguments the attribute is a property of the call location.
>   
> -If the represented attribute expects a constant argument, the argument provided
> -to the operand bundle should be a constant as well.
> -
>   For example:
>   
>   .. code-block:: llvm
> @@ -2450,6 +2447,20 @@ call location is cold and that ``%val`` may not be null.
>   Just like for the argument of :ref:`llvm.assume <int_assume>`, if any of the
>   provided guarantees are violated at runtime the behavior is undefined.
>   
> +While attributes expect constant arguments, assume operand bundles may be
> +provided a dynamic value, for example:
> +
> +.. code-block:: llvm
> +
> +      call void @llvm.assume(i1 true) ["align"(i32* %val, i32 %align)]
> +
> +If the operand bundle value violates any requirements on the attribute value,
> +the behavior is undefined, unless one of the following exceptions applies:
> +
> +* ``"assume"`` operand bundles may specify a non-power-of-two alignment
> +  (including a zero alignment). If this is the case, then the pointer value
> +  must be a null pointer, otherwise the behavior is undefined.
> +
>   Even if the assumed property can be encoded as a boolean value, like
>   ``nonnull``, using operand bundles to express the property can still have
>   benefits:
>
>
>          
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits