Skip to content

[LangRef/DataLayout] Spell out requirements for alignment values #104705

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.

Sign up for GitHub

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 6 commits into from
Apr 6, 2025
Merged

[LangRef/DataLayout] Spell out requirements for alignment values #104705

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7c87913
[LangRef/DataLayout] Spell out requirements for alignment values
s-barannikov Aug 15, 2024
b5150ad
fix typo
s-barannikov Aug 18, 2024
416c78c
Try to fix CI error
s-barannikov Aug 18, 2024
2ffb292
Avoid text duplication
s-barannikov Aug 18, 2024
27a55a4
Minor wording change
s-barannikov Aug 18, 2024
241ffc3
Merge branch 'main' into datalayout/langref-alignments
s-barannikov Apr 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions llvm/docs/LangRef.rst
View file
Open in desktop
Copy link
Contributor

@nikic nikic Aug 18, 2024

Choose a reason for hiding this comment

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

Can we say once at the top that all alignment specs must be powers of two etc, rather than repeating it for each use?

Copy link
Contributor Author

@s-barannikov s-barannikov Aug 18, 2024

Choose a reason for hiding this comment

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

I did that because size and address spaces requirements are repeated too, but I don't feel like I should be moving them in this PR. (Or should I?)

Copy link
Contributor Author

@s-barannikov s-barannikov Aug 18, 2024

Choose a reason for hiding this comment

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

I've moved the repeated sentences to the bottom, next to the other text that talks about alignments.

Original file line number Diff line number Diff line change
Expand Up @@ -3030,8 +3030,10 @@ as follows:
``S<size>``
Specifies the natural alignment of the stack in bits. Alignment
promotion of stack variables is limited to the natural stack
alignment to avoid dynamic stack realignment. The stack alignment
must be a multiple of 8-bits. If omitted, the natural stack
Copy link

@tschuett tschuett Aug 18, 2024

Choose a reason for hiding this comment

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

Please do not delete the 8-bits!!!

Copy link
Contributor Author

@s-barannikov s-barannikov Aug 18, 2024

Choose a reason for hiding this comment

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

It is implied in the added paragraph.

Copy link

@tschuett tschuett Aug 18, 2024

Choose a reason for hiding this comment

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

This is an 8-bit per byte world and your downstream target differs from that assumption. Don't play games.

Copy link
Contributor Author

@s-barannikov s-barannikov Aug 18, 2024

Choose a reason for hiding this comment

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

Please be constructive in your criticism.
I'm marking this conversation as resolved unless you have any specific concerns.

Copy link

@tschuett tschuett Aug 18, 2024

Choose a reason for hiding this comment

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

Not resolved. You substitute 8-bit by byte to remove the 8-bit restriction.

Copy link

@tschuett tschuett Aug 18, 2024

Choose a reason for hiding this comment

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

constructive criticism?

Copy link

@tschuett tschuett Aug 18, 2024

Choose a reason for hiding this comment

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

cheap trick

Copy link
Contributor

@nikic nikic Aug 20, 2024

Choose a reason for hiding this comment

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

I think it's fine omit the 8-bit mention here. It's only mentioned once for S and not any of the other alignment specifications. I looked through LangRef a bit, and with the notable exception of this sentence, the wording always tries to be byte-size agnostic.

Copy link
Contributor

@nikic nikic Aug 20, 2024

Choose a reason for hiding this comment

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

LangRef probably should mention somewhere that bytes are 8 bits large in LLVM -- this just isn't the right place, as it's a global assumption, not specific to only this one thing.

Not sure it makes sense to actually add that note now though, if there are plans to lift this restriction in the near future.

alignment to avoid dynamic stack realignment.
The stack alignment must be must be in the range [1,2^16)
and must be a power of two times the byte width.
Copy link
Contributor Author

@s-barannikov s-barannikov Aug 18, 2024

Choose a reason for hiding this comment

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

I'm not very happy with this wording.
What I'm trying to say is that the value must be ByteWidth * 2^N ,without assuming that ByteWidth == 8.

If omitted, the natural stack
alignment defaults to "unspecified", which does not prevent any
alignment promotions.
``P<address space>``
Expand Down Expand Up @@ -3065,26 +3067,37 @@ as follows:
are in bits. The address space, ``n``, is optional, and if not specified,
denotes the default address space 0. The value of ``n`` must be
in the range [1,2^24).
The values of ``<abi>`` and ``<pref>`` must be in the range [1,2^16)
and must be powers of two times the byte width.
``i<size>:<abi>[:<pref>]``
This specifies the alignment for an integer type of a given bit
``<size>``. The value of ``<size>`` must be in the range [1,2^24).
``<pref>`` is optional and defaults to ``<abi>``.
The values of ``<abi>`` and ``<pref>`` must be in the range [1,2^16)
and must be powers of two times the byte width.
For ``i8``, the ``<abi>`` value must equal 8,
that is, ``i8`` must be naturally aligned.
``v<size>:<abi>[:<pref>]``
This specifies the alignment for a vector type of a given bit
``<size>``. The value of ``<size>`` must be in the range [1,2^24).
``<pref>`` is optional and defaults to ``<abi>``.
The values of ``<abi>`` and ``<pref>`` must be in the range [1,2^16)
and must be powers of two times the byte width.
``f<size>:<abi>[:<pref>]``
This specifies the alignment for a floating-point type of a given bit
``<size>``. Only values of ``<size>`` that are supported by the target
will work. 32 (float) and 64 (double) are supported on all targets; 80
or 128 (different flavors of long double) are also supported on some
targets. The value of ``<size>`` must be in the range [1,2^24).
``<pref>`` is optional and defaults to ``<abi>``.
The values of ``<abi>`` and ``<pref>`` must be in the range [1,2^16)
and must be powers of two times the byte width.
``a:<abi>[:<pref>]``
This specifies the alignment for an object of aggregate type.
``<pref>`` is optional and defaults to ``<abi>``.
The values of ``<abi>`` and ``<pref>`` must be in the range [1,2^16)
and must be powers of two times the byte width.
The value of ``<<abi>>` may also be zero meaning one byte alignment.
``F<type><abi>``
This specifies the alignment for function pointers.
The options for ``<type>`` are:
Expand All @@ -3093,6 +3106,8 @@ as follows:
of functions, and is a multiple of ``<abi>``.
* ``n``: The alignment of function pointers is a multiple of the explicit
alignment specified on the function, and is a multiple of ``<abi>``.
The values of ``<abi>`` and ``<pref>`` must be in the range [1,2^16)
and must be powers of two times the byte width.
``m:<mangling>``
If present, specifies that llvm names are mangled in the output. Symbols
prefixed with the mangling escape character ``\01`` are passed through
Expand Down