-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[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
Conversation
Update the documentation for the noalias attribute, !alias.scope and !loop.parallel_accesses metadata to clarify they are UB on voilation the noalias property.
|
@llvm/pr-subscribers-llvm-ir Author: Florian Hahn (fhahn) ChangesUpdate 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:
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
|
nunoplopes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Just realized that I forgot to update the !tbaa metadata, which probably also should get the same treatment |
|
Trying to remind myself why exactly using "load is poison, store is UB" semantics wouldn't work: If we have where we have claimed that these are noalias, then the If we now move the load above the store (because they don't alias) then 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. |
nikic
left a comment
There was a problem hiding this 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.
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
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
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
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.
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.
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.
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.
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.
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.
…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
…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)
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)
…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)
Update the documentation for the noalias attribute, !alias.scope and !loop.parallel_accesses metadata to clarify they are UB on voilation the noalias property.