Skip to content
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

[docs] Reword the alignment implications for atomic instructions. #75871

Merged
merged 2 commits into from
Dec 20, 2023

Conversation

JonPsson1
Copy link
Contributor

Atomic instructions (load / store/ atomicrwm / cmpxchg) are not undefined behavior if they lack natural alignment. They will typically (with AtomicExpand pass enabled) be converted into libcalls.

Does this sound right? (suggestions welcome)

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 18, 2023

@llvm/pr-subscribers-llvm-ir

Author: Jonas Paulsson (JonPsson1)

Changes

Atomic instructions (load / store/ atomicrwm / cmpxchg) are not undefined behavior if they lack natural alignment. They will typically (with AtomicExpand pass enabled) be converted into libcalls.

Does this sound right? (suggestions welcome)


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

1 Files Affected:

  • (modified) llvm/docs/LangRef.rst (+13-8)
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 7f4a316a21acee..326b2c29ff38c6 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -10515,9 +10515,10 @@ Atomic loads produce :ref:`defined <memmodel>` results when they may see
 multiple atomic stores. The type of the pointee must be an integer, pointer, or
 floating-point type whose bit width is a power of two greater than or equal to
 eight and less than or equal to a target-specific size limit.  ``align`` must be
-explicitly specified on atomic loads, and the load has undefined behavior if the
-alignment is not set to a value which is at least the size in bytes of the
-pointee. ``!nontemporal`` does not have any defined semantics for atomic loads.
+explicitly specified on atomic loads and must typically be a power of two greater
+or equal to the size of the `<value>` type, or the AtomicExpand pass will convert
+it to a libcall. ``!nontemporal`` does not have any defined semantics for atomic
+loads.
 
 The optional constant ``align`` argument specifies the alignment of the
 operation (that is, the alignment of the memory address). It is the
@@ -10655,7 +10656,11 @@ Atomic loads produce :ref:`defined <memmodel>` results when they may see
 multiple atomic stores. The type of the pointee must be an integer, pointer, or
 floating-point type whose bit width is a power of two greater than or equal to
 eight and less than or equal to a target-specific size limit.  ``align`` must be
-explicitly specified on atomic stores, and the store has undefined behavior if
+explicitly specified on atomic stores and must typically be a power of two
+greater or equal to the size of the `<value>` type, or the AtomicExpand pass will
+convert it to a libcall.
+
+and the store has undefined behavior if
 the alignment is not set to a value which is at least the size in bytes of the
 pointee. ``!nontemporal`` does not have any defined semantics for atomic stores.
 
@@ -10807,8 +10812,8 @@ must be at least ``monotonic``, the failure ordering cannot be either
 A ``cmpxchg`` instruction can also take an optional
 ":ref:`syncscope <syncscope>`" argument.
 
-The alignment must be a power of two greater or equal to the size of the
-`<value>` type.
+Typically, the alignment must be a power of two greater or equal to the size
+of the `<value>` type, or the AtomicExpand pass will convert it to a libcall.
 
 The alignment is only optional when parsing textual IR; for in-memory IR, it is
 always present. If unspecified, the alignment is assumed to be equal to the
@@ -10910,8 +10915,8 @@ the ``atomicrmw`` is marked as ``volatile``, then the optimizer is not
 allowed to modify the number or order of execution of this
 ``atomicrmw`` with other :ref:`volatile operations <volatile>`.
 
-The alignment must be a power of two greater or equal to the size of the
-`<value>` type.
+Typically, the alignment must be a power of two greater or equal to the size
+of the `<value>` type, or the AtomicExpand pass will convert it to a libcall.
 
 The alignment is only optional when parsing textual IR; for in-memory IR, it is
 always present. If unspecified, the alignment is assumed to be equal to the

@JonPsson1
Copy link
Contributor Author

Adding some more reviewers so that hopefully there can be some consensus around this notion whether an unaligned atomic IR instruction is undefined behavior or not. This is motivated by the fact that AtomicExpand pass always turn these into libcalls.

@jyknight
Copy link
Member

Overall, yes.

I don't think the mention of AtomicExpandPass belongs here. It should instead just say something like "Note: if the alignment is not greater than the size, the atomic operation is likely to require a lock and have poor performance."

@JonPsson1
Copy link
Contributor Author

Yeah, that's probably better. Patch updated.

Copy link
Member

@jyknight jyknight left a comment

Choose a reason for hiding this comment

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

Thanks!

@JonPsson1 JonPsson1 merged commit f94adfd into llvm:main Dec 20, 2023
4 of 5 checks passed
@JonPsson1 JonPsson1 deleted the LangRefAtomicAlign branch December 20, 2023 23:08
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.

None yet

3 participants