[LangRef] Make !range, !nonnull and !align return poison instead of IUB

[nikic]

Summary

Make violation of !range, !nonnull and !align metadata return poison instead of causing immediate undefined behavior. This makes the behavior match that of the nonnull and align parameter and return value attributes. The previous behavior can be restored by additionally specifying the !noundef metadata, same as with parameters.

Some benefits of this change are:

• This is needed to fix https://github.com/llvm/llvm-project/issues/59888. Under current semantics, it is illegal to add !range annotations based on known bits. Unless we want to drop that optimization entirely, we need to change the !range semantics.
• This allows preserving range/nonnull/align metadata on speculated loads. !noundef metadata needs to be dropped, but the poison-generating metadata can be retained.

I don't think there are really disadvantages to the change (apart from the need to review and adjust optimizations for the new semantics), as the old behavior is still available via !noundef, so it should be strictly more flexible.

[jdoerfert]

+1. This is what we have (slowly) been doing elsewhere and it works. Thanks!

[nlopes]

Sounds good, thanks!
I implemented this in Alive2 and I got zero failures in LLVM's test suite. Either it doesn't have many !ranges or we are good.

[nikic]

In D141386#4044303, @nlopes wrote:

Sounds good, thanks!
I implemented this in Alive2 and I got zero failures in LLVM's test suite. Either it doesn't have many !ranges or we are good.

Shouldn't this test have failed? https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/Mem2Reg/preserve-nonnull-load-metadata.ll Or did you only adjust !range but not !nonnull?

[nlopes]

In D141386#4047344, @nikic wrote:

In D141386#4044303, @nlopes wrote:

Sounds good, thanks!
I implemented this in Alive2 and I got zero failures in LLVM's test suite. Either it doesn't have many !ranges or we are good.

Shouldn't this test have failed? https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/Mem2Reg/preserve-nonnull-load-metadata.ll Or did you only adjust !range but not !nonnull?

It seems it's not even implemented 😅:

ERROR: Unsupported metadata: 11
ERROR: Could not translate 'single_store' to Alive IR

Ok, I'll add it to my todo list. maybe over the weekend..

[nlopes]

In D141386#4047344, @nikic wrote:

In D141386#4044303, @nlopes wrote:

Sounds good, thanks!
I implemented this in Alive2 and I got zero failures in LLVM's test suite. Either it doesn't have many !ranges or we are good.

Shouldn't this test have failed? https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/Mem2Reg/preserve-nonnull-load-metadata.ll Or did you only adjust !range but not !nonnull?

It does now:

define ptr @no_store_single_load() {
%entry:
  %buf = alloca i64 8, align 8
  %buf.load = load ptr, ptr %buf, align 8
  %buf.load_2 = !nonnull ptr %buf.load
  ret ptr %buf.load_2
}
=>
define ptr @no_store_single_load() noreturn nonnull nofree noalias willreturn memory(none) {
%entry:
  assume i1 0
}
Transformation doesn't verify!

ERROR: Source is more defined than target

I'll rerun LLVM unit tests again.

[nlopes]

Three failures:

• Transforms/InstCombine/loadstore-metadata.ll
• Transforms/Mem2Reg/preserve-nonnull-load-metadata.ll
• Transforms/SROA/preserve-nonnull.ll

[nikic]

In D141386#4048676, @nlopes wrote:

Three failures:

• Transforms/InstCombine/loadstore-metadata.ll
• Transforms/Mem2Reg/preserve-nonnull-load-metadata.ll
• Transforms/SROA/preserve-nonnull.ll

The last two should be fixed by https://github.com/llvm/llvm-project/commit/e6241cbdcbf3cc9beb49460578466e18936ef220.

For the InstCombine test the online alive instance currently crashes (https://alive2.llvm.org/ce/z/dZko2g), so not sure what's wrong with that one.

[nlopes]

In D141386#4069054, @nikic wrote:

In D141386#4048676, @nlopes wrote:

Three failures:

• Transforms/InstCombine/loadstore-metadata.ll
• Transforms/Mem2Reg/preserve-nonnull-load-metadata.ll
• Transforms/SROA/preserve-nonnull.ll

The last two should be fixed by https://github.com/llvm/llvm-project/commit/e6241cbdcbf3cc9beb49460578466e18936ef220.

Thank you!

For the InstCombine test the online alive instance currently crashes (https://alive2.llvm.org/ce/z/dZko2g), so not sure what's wrong with that one.

It doesn't like this: !nonnull !6.
It is failing the assertion that !nonnull has no arguments. I would hope the IR verifier would catch this though.

[nikic]

In D141386#4069084, @nlopes wrote:

In D141386#4069054, @nikic wrote:

In D141386#4048676, @nlopes wrote:

Three failures:

• Transforms/InstCombine/loadstore-metadata.ll
• Transforms/Mem2Reg/preserve-nonnull-load-metadata.ll
• Transforms/SROA/preserve-nonnull.ll

The last two should be fixed by https://github.com/llvm/llvm-project/commit/e6241cbdcbf3cc9beb49460578466e18936ef220.

Thank you!

For the InstCombine test the online alive instance currently crashes (https://alive2.llvm.org/ce/z/dZko2g), so not sure what's wrong with that one.

It doesn't like this: !nonnull !6.
It is failing the assertion that !nonnull has no arguments. I would hope the IR verifier would catch this though.

I've added the verifier check in https://github.com/llvm/llvm-project/commit/474f20ba26400559e5d99fd4f29926253092f00b.

llvm/docs/LangRef.rst

 .. _range-metadata:
 
 '``range``' Metadata
 ^^^^^^^^^^^^^^^^^^^^
 
 ``range`` metadata may be attached only to ``load``, ``call`` and ``invoke`` of
 integer types. It expresses the possible ranges the loaded value or the value
 returned by the called function at this call site is in. If the loaded or
-returned value is not in the specified range, the behavior is undefined. The
-ranges are represented with a flattened list of integers. The loaded value or
-the value returned is known to be in the union of the ranges defined by each
-consecutive pair. Each pair has the following properties:
+returned value is not in the specified range, a poison value is returned
+instead. The ranges are represented with a flattened list of integers. The
+loaded value or the value returned is known to be in the union of the ranges
+defined by each consecutive pair. Each pair has the following properties:
 
 -  The type must match the type loaded by the instruction.
 -  The pair ``a,b`` represents the range ``[a,b)``.
 -  Both ``a`` and ``b`` are constants.
 -  The range is allowed to wrap.
 -  The range should not represent the full or empty set. That is,
    ``a!=b``.
 

 The optional ``!invariant.group`` metadata must reference a single metadata name
  ``<empty_node>`` corresponding to a metadata node with no entries.
  See ``invariant.group`` metadata :ref:`invariant.group <md_invariant.group>`.
 
 The optional ``!nonnull`` metadata must reference a single
 metadata name ``<empty_node>`` corresponding to a metadata node with no
 entries. The existence of the ``!nonnull`` metadata on the
 instruction tells the optimizer that the value loaded is known to
-never be null. If the value is null at runtime, the behavior is undefined.
-This is analogous to the ``nonnull`` attribute on parameters and return
-values. This metadata can only be applied to loads of a pointer type.
+never be null. If the value is null at runtime, a poison value is returned
+instead.  This is analogous to the ``nonnull`` attribute on parameters and
+return values. This metadata can only be applied to loads of a pointer type.
 
 The optional ``!dereferenceable`` metadata must reference a single metadata
 name ``<deref_bytes_node>`` corresponding to a metadata node with one ``i64``
 entry.
 See ``dereferenceable`` metadata :ref:`dereferenceable <md_dereferenceable>`.
 
 The optional ``!dereferenceable_or_null`` metadata must reference a single
 metadata name ``<deref_bytes_node>`` corresponding to a metadata node with one
 ``i64`` entry.
 See ``dereferenceable_or_null`` metadata :ref:`dereferenceable_or_null
 <md_dereferenceable_or_null>`.
 
 The optional ``!align`` metadata must reference a single metadata name
 ``<align_node>`` corresponding to a metadata node with one ``i64`` entry.
 The existence of the ``!align`` metadata on the instruction tells the
 optimizer that the value loaded is known to be aligned to a boundary specified
 by the integer value in the metadata node. The alignment must be a power of 2.
 This is analogous to the ''align'' attribute on parameters and return values.
 This metadata can only be applied to loads of a pointer type. If the returned
-value is not appropriately aligned at runtime, the behavior is undefined.
+value is not appropriately aligned at runtime, a poison value is returned
+instead.
 
 The optional ``!noundef`` metadata must reference a single metadata name
 ``<empty_node>`` corresponding to a node with no entries. The existence of
 ``!noundef`` metadata on the instruction tells the optimizer that the value
 loaded is known to be :ref:`well defined <welldefinedvalues>`.
-If the value isn't well defined, the behavior is undefined.
+If the value isn't well defined, the behavior is undefined. If the ``!noundef``
+metadata is combined with poison-generating metadata like ``!nonnull``,
+violation of that metadata constraint will also result in undefined behavior.
 
 Semantics:
 """"""""""
 
 The location of memory pointed to is loaded. If the value being loaded
 is of scalar type then the number of bytes read does not exceed the
 minimum number of bytes needed to hold all bits of the type. For
 example, loading an ``i24`` reads at most three bytes. When loading a