Skip to content

[LangRef] Spell out alias attribute/metadata violations are UB. #116220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 16, 2024

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Nov 14, 2024

Update the documentation for the noalias attribute, !alias.scope and !loop.parallel_accesses metadata to clarify they are UB on voilation the noalias property.

Update the documentation for the noalias attribute, !alias.scope and
!loop.parallel_accesses metadata to clarify they are UB on voilation the
noalias property.
@llvmbot
Copy link
Member

llvmbot commented Nov 14, 2024

@llvm/pr-subscribers-llvm-ir

Author: Florian Hahn (fhahn)

Changes

Update the documentation for the noalias attribute, !alias.scope and !loop.parallel_accesses metadata to clarify they are UB on voilation the noalias property.


Full diff: https://github.com/llvm/llvm-project/pull/116220.diff

1 Files Affected:

  • (modified) llvm/docs/LangRef.rst (+13-10)
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 6fb66ce231e8ab..6183f0f404f446 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -1358,13 +1358,15 @@ Currently, only the following parameter attributes are defined:
     This indicates that memory locations accessed via pointer values
     :ref:`based <pointeraliasing>` on the argument or return value are not also
     accessed, during the execution of the function, via pointer values not
-    *based* on the argument or return value. This guarantee only holds for
+    *based* on the argument or return value.This guarantee only holds for
     memory locations that are *modified*, by any means, during the execution of
-    the function. The attribute on a return value also has additional semantics
-    described below. The caller shares the responsibility with the callee for
-    ensuring that these requirements are met.  For further details, please see
-    the discussion of the NoAlias response in :ref:`alias analysis <Must, May,
-    or No>`.
+    the function. If there are other accesses not based on the argument or
+    return value, the behavior is undefined. The attribute on a return value
+    also has additional semantics described below. The caller shares the
+    responsibility with the callee for described below. The caller shares the
+    responsibility with the callee for ensuring that these requirements are met.
+    For further details, please see the discussion of the NoAlias response in
+    :ref:`alias analysis <Must, May,  or No>`.
 
     Note that this definition of ``noalias`` is intentionally similar
     to the definition of ``restrict`` in C99 for function arguments.
@@ -6936,9 +6938,9 @@ does not carry useful data and need not be preserved.
 noalias memory-access sets. This means that some collection of memory access
 instructions (loads, stores, memory-accessing calls, etc.) that carry
 ``noalias`` metadata can specifically be specified not to alias with some other
-collection of memory access instructions that carry ``alias.scope`` metadata.
-Each type of metadata specifies a list of scopes where each scope has an id and
-a domain.
+collection of memory access instructions that carry ``alias.scope`` metadata. If
+accesses from different collections alias, the behaivor is undefined. Each type
+of metadata specifies a list of scopes where each scope has an id and a domain.
 
 When evaluating an aliasing query, if for some domain, the set
 of scopes with that domain in one instruction's ``alias.scope`` list is a
@@ -7695,7 +7697,8 @@ If all memory-accessing instructions in a loop have
 ``llvm.access.group`` metadata that each refer to one of the access
 groups of a loop's ``llvm.loop.parallel_accesses`` metadata, then the
 loop has no loop carried memory dependencies and is considered to be a
-parallel loop.
+parallel loop. If there is a loop-carried dependency, the behavior is
+undefined.
 
 Note that if not all memory access instructions belong to an access
 group referred to by ``llvm.loop.parallel_accesses``, then the loop must

Copy link
Member

@nunoplopes nunoplopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fhahn
Copy link
Contributor Author

fhahn commented Nov 14, 2024

Just realized that I forgot to update the !tbaa metadata, which probably also should get the same treatment

@nikic
Copy link
Contributor

nikic commented Nov 14, 2024

Trying to remind myself why exactly using "load is poison, store is UB" semantics wouldn't work:

If we have

store v2, p
v = load p

where we have claimed that these are noalias, then the store is well-defined and the load returns poison (due to conflicting store).

If we now move the load above the store (because they don't alias)

v = load p
store v2, p

then load returns a value and store is UB due to conflicting prior store. So we went from poison to UB.

So I believe that for scoped alias metadata only UB semantics can work, as specified here. Presumably a similar argument generalizes to the other metadata.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please wait a bit in case there are more comments.

@fhahn fhahn merged commit c95daac into llvm:main Nov 16, 2024
7 of 9 checks passed
@fhahn fhahn deleted the langref-alias-md-ub branch November 16, 2024 13:39
fhahn added a commit to fhahn/llvm-project that referenced this pull request Nov 18, 2024
Preserve tbaa metadata on the replacement instruction, if it does not
move. In that case, the program would be UB, if the aliasing property
encoded in the metadata does not hold.

This makes use of the clarification re tbaa metadata implying UB if the
property does not hold: llvm#116220
fhahn added a commit that referenced this pull request Nov 19, 2024
…15868)

Preserve llvm.access.group metadata on the replacement instruction, if
it does not move. In that case, the program would be UB, if the parallel
property encoded in the metadata does not hold.

This matches the LangRef recently updated in #116220

PR #115868
fhahn added a commit to fhahn/llvm-project that referenced this pull request Nov 20, 2024
Preserve tbaa metadata on the replacement instruction, if it does not
move. In that case, the program would be UB, if the aliasing property
encoded in the metadata does not hold.

This makes use of the clarification re tbaa metadata implying UB if the
property does not hold: llvm#116220
fhahn added a commit that referenced this pull request Nov 20, 2024
Preserve tbaa metadata on the replacement instruction, if it does not
move. In that case, the program would be UB, if the aliasing property
encoded in the metadata does not hold.

This makes use of the clarification re tbaa metadata implying UB if the
property does not hold: #116220

Same as #115868, but for !tbaa

PR: #116682
fhahn added a commit that referenced this pull request Nov 21, 2024
The new test illustrates a case where LICM introduces UB-implying
metadata on speculated loads that may not execute in the original
version.

The aliasing metadata behavior has been clarified in
 #116220.
fhahn added a commit to fhahn/llvm-project that referenced this pull request Nov 21, 2024
llvm#116220 clarified that
violations of aliasing metadata are UB.

Only set the AA metadata after hoisting a log, if it is guaranteed to
execute in the original loop.
nikic pushed a commit to nikic/llvm-project that referenced this pull request Nov 22, 2024
llvm#116220 clarified that
violations of aliasing metadata are UB.

Only set the AA metadata after hoisting a log, if it is guaranteed to
execute in the original loop.
fhahn added a commit to fhahn/llvm-project that referenced this pull request Nov 23, 2024
llvm#116220 clarified that
violations of aliasing metadata are UB.

Only set the AA metadata after hoisting a log, if it is guaranteed to
execute in the original loop.
fhahn added a commit to fhahn/llvm-project that referenced this pull request Nov 26, 2024
llvm#116220 clarified that
violations of aliasing metadata are UB.

Only set the AA metadata after hoisting a log, if it is guaranteed to
execute in the original loop.
fhahn added a commit to fhahn/llvm-project that referenced this pull request Nov 26, 2024
Preserve !alias.scope, !noalias and !mem.parallel_loop_access metadata
on the replacement instruction, if it does not move. In that case, the
program would be UB, if the aliasing property encoded in the metadata
does not hold. This makes use of the clarification re aliasing metadata
implying UB if the property does not hold: llvm#116220

Same as llvm#115868, but for !alias.scope, !noalias and
!mem.parallel_loop_access.
fhahn added a commit that referenced this pull request Nov 26, 2024
#116220 clarified that
violations of aliasing metadata are UB.

Only set the AA metadata after hoisting a log, if it is guaranteed to
execute in the original loop.

PR: #117204
fhahn added a commit that referenced this pull request Nov 26, 2024
…ves (#117716)

Preserve !alias.scope, !noalias and !mem.parallel_loop_access metadata
on the replacement instruction, if it does not move. In that case, the
program would be UB, if the aliasing property encoded in the metadata
does not hold. This makes use of the clarification re aliasing metadata
implying UB if the property does not hold: #116220

Same as #115868, but for !alias.scope, !noalias and
!mem.parallel_loop_access.


PR: #117716
fhahn added a commit to fhahn/llvm-project that referenced this pull request Jan 9, 2025
…vm#115868)

Preserve llvm.access.group metadata on the replacement instruction, if
it does not move. In that case, the program would be UB, if the parallel
property encoded in the metadata does not hold.

This matches the LangRef recently updated in llvm#116220

PR llvm#115868

(cherry picked from commit 0765136)
fhahn added a commit to fhahn/llvm-project that referenced this pull request Jan 9, 2025
Preserve tbaa metadata on the replacement instruction, if it does not
move. In that case, the program would be UB, if the aliasing property
encoded in the metadata does not hold.

This makes use of the clarification re tbaa metadata implying UB if the
property does not hold: llvm#116220

Same as llvm#115868, but for !tbaa

PR: llvm#116682
(cherry picked from commit 0bb1b68)
fhahn added a commit to fhahn/llvm-project that referenced this pull request Jan 9, 2025
…ves (llvm#117716)

Preserve !alias.scope, !noalias and !mem.parallel_loop_access metadata
on the replacement instruction, if it does not move. In that case, the
program would be UB, if the aliasing property encoded in the metadata
does not hold. This makes use of the clarification re aliasing metadata
implying UB if the property does not hold: llvm#116220

Same as llvm#115868, but for !alias.scope, !noalias and
!mem.parallel_loop_access.

PR: llvm#117716
(cherry picked from commit 46a0857)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants