This is an archive of the discontinued LLVM Phabricator instance.

Differential D108962

[docs] [NFC] Clarify the datalayout documentation
ClosedPublic

Authored by woodruffw on Aug 30 2021, 6:07 PM.

Details

Reviewers
aqjune
fhahn
t.p.northover
xgupta
Summary

This patch fixes a couple of small oversights in the documentation for the datalayout specification:

  • The v and f specifications are subject to the same constraints on <size> as i is.
  • The p[n] specification didn't mark <idx> as optional, despite being documented and parsed as such.
  • Similarly, none of the alignment specifications require <pref>.

Diff Detail

Unit TestsFailed

TimeTest
16,370 msx64 debian > libarcher.races::task-dependency.c

Event Timeline

woodruffw created this revision.Aug 30 2021, 6:07 PM
Herald added a subscriber: jdoerfert. · View Herald TranscriptAug 30 2021, 6:07 PM
woodruffw requested review of this revision.Aug 30 2021, 6:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 30 2021, 6:07 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B121856: Diff 369593.Aug 30 2021, 6:08 PM
woodruffw updated this revision to Diff 369616.Aug 30 2021, 8:39 PM
woodruffw edited the summary of this revision. (Show Details)

Also clarify that <pref> is optional in pointer alignment specifications.

woodruffw updated this revision to Diff 369618.Aug 30 2021, 8:53 PM
woodruffw edited the summary of this revision. (Show Details)

Mark <pref> as optional in all of the alignment specs, since the DataLayout parser treats it as optional.

Harbormaster completed remote builds in B121877: Diff 369618.Aug 30 2021, 9:30 PM
woodruffw added a comment.Sep 7 2021, 11:39 AM

Ping for review on this.

woodruffw added a comment.Sep 14 2021, 10:54 AM

Another gentle ping for review.

woodruffw added a reviewer: Restricted Project.Oct 3 2021, 7:57 PM
xgupta edited reviewers, added: aqjune, fhahn; removed: Restricted Project.Oct 8 2021, 8:20 PM
fhahn added a reviewer: t.p.northover.Oct 11 2021, 3:37 AM
fhahn added a subscriber: t.p.northover.

Looks good to me in general, thanks for updating the docs! Perhaps @t.p.northover has any further suggestions?

nikic added a subscriber: nikic.Oct 11 2021, 3:43 AM
nikic added inline comments.
llvm/docs/LangRef.rst
2563

Pre-existing, but there seems to be some words missing here. "is the size of the index that is used"?

woodruffw updated this revision to Diff 378748.Oct 11 2021, 11:46 AM

Fixed awkward language.

llvm/docs/LangRef.rst
2563

Yeah, that reads awkwardly to me. Fixed.

Harbormaster completed remote builds in B128167: Diff 378748.Oct 11 2021, 1:25 PM
xgupta accepted this revision.Oct 12 2021, 5:16 AM
This revision is now accepted and ready to land.Oct 12 2021, 5:16 AM
xgupta closed this revision.Oct 12 2021, 10:53 AM

committed in https://reviews.llvm.org/rG86a4a93a1c444f297c2dd92815b4bf9093dfde8e. Thanks @woodruffw.

Revision Contents

PathSize
llvm/
docs/
6 lines

Diff 369593

llvm/docs/LangRef.rst

Context not available.
``A<address space>`` ``A<address space>``
Specifies the address space of objects created by '``alloca``'. Specifies the address space of objects created by '``alloca``'.
Defaults to the default address space of 0. Defaults to the default address space of 0.
``p[n]:<size>:<abi>:<pref>:<idx>`` ``p[n]:<size>:<abi>:<pref>[:<idx>]``
This specifies the *size* of a pointer and its ``<abi>`` and This specifies the *size* of a pointer and its ``<abi>`` and
``<pref>``\erred alignments for address space ``n``. The fourth parameter ``<pref>``\erred alignments for address space ``n``. The fourth parameter
``<idx>`` is a size of index that used for address calculation. If not ``<idx>`` is a size of index that used for address calculation. If not
nikicUnsubmitted
Not Done
ReplyInline Actions

Pre-existing, but there seems to be some words missing here. "is the size of the index that is used"?

nikic: Pre-existing, but there seems to be some words missing here. "is the size of the index that is…
woodruffwAuthorUnsubmitted
Done
ReplyInline Actions

Yeah, that reads awkwardly to me. Fixed.

woodruffw: Yeah, that reads awkwardly to me. Fixed.
Context not available.
``<size>``. The value of ``<size>`` must be in the range [1,2^23). ``<size>``. The value of ``<size>`` must be in the range [1,2^23).
``v<size>:<abi>:<pref>`` ``v<size>:<abi>:<pref>``
This specifies the alignment for a vector type of a given bit This specifies the alignment for a vector type of a given bit
``<size>``. ``<size>``. The value of ``<size>`` must be in the range [1,2^23).
``f<size>:<abi>:<pref>`` ``f<size>:<abi>:<pref>``
This specifies the alignment for a floating-point type of a given bit This specifies the alignment for a floating-point type of a given bit
``<size>``. Only values of ``<size>`` that are supported by the target ``<size>``. Only values of ``<size>`` that are supported by the target
will work. 32 (float) and 64 (double) are supported on all targets; 80 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 or 128 (different flavors of long double) are also supported on some
targets. targets. The value of ``<size>`` must be in the range [1,2^23).
``a:<abi>:<pref>`` ``a:<abi>:<pref>``
This specifies the alignment for an object of aggregate type. This specifies the alignment for an object of aggregate type.
``F<type><abi>`` ``F<type><abi>``
Context not available.