Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
364463002:
Initial draft of PNaCl bitcode files. (Closed)
|
Created:
6 years, 5 months ago by Karl Modified:
6 years, 1 month ago CC:
chromium-reviews, binji+watch_chromium.org, chromium-apps-reviews_chromium.org, native-client-reviews_googlegroups.com, Sam Clegg, extensions-reviews_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Project:
chromium Visibility:
Public. |
DescriptionInitial draft of PNaCl bitcode files.
Focus of this CL is on the PNaCl records that can appear in bitcode files.
A subsequent CL will add how the records are converted to bitcode sequences.
BUG=None
Patch Set 1 #Patch Set 2 : Next round of cleanups. #
Total comments: 366
Messages
Total messages: 8 (0 generated)
Replaces CL https://codereview.chromium.org/266713003 which was too old to properly update.
I think this document is complete enough that we should add it to our web pages. Please review this with that in mind. Thanks.
Initial spellcheck. :) Deeper review in progress. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... File native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst (right): https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:291: the internet. Internet? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:353: terms of data within the globa state and the corresponding syntax. It also global https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:376: are literal while the upper case sequence of alphanumeric charaters denote rule characters https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:492: bitsequences using default abbreviations. bit sequences https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:535: On loads and stores, the aligment in the instruction is used to communicate what alignment https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:539: use that information to potentially chose a more efficent sequence of efficient https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:586: calling type constraints as ordinary functions. That is, an instrisic can use intrinsic https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:627: represent the function addrress which is a pointer (and pointers are alwyas address https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:627: represent the function addrress which is a pointer (and pointers are alwyas always https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1010: corresponding type identifer in the types block. Hence, the requirement that the identifier https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1338: types are used when multiple primitve data are operated in parallel using a primitive https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1443: allowed. For intrisic functions, all integral types are allowed for both return and intrinsic https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1588: Global Variable Addressses Addresses https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1731: A zerofill initializer record intializes a sequence of bytes, associated with a initializes https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2044: **Semnatics** Semantics https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2047: sequence of anscii characters *B1* through *BN*. ascii or ASCII https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2220: Decribes the function address *@fN*. *PN* is the name that specifies the Describes https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2368: The *set type* record deifnes type *T* to be used to type the (immediately) defines https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2422: The *undefined* lieral record creates an undefined literal constant *%cN* for literal https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2494: T == ConsgtantsSetType ConstantsSetType? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3131: places, based on a selector value. It is a generaliation of the conditional generalization https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3152: **Sematics** Semantics https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3418: In the subtract instruction, integral type i1 (or a vector on integrap type i1) integral https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3489: negative integer by -1. The behaviour for this case is undefined. behavior :) https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3747: the result *%vN* must be of type *T*. *T* nust be an integral, or a vector of must https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3815: *V2* and the result *%vN* must be of type *T*. *T* nust be an integral, or a must https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3882: and *V2* and and the result *%vN* must be of type *T*. *T* nust be an integral, must https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3893: integrl type i1) is disallowed. integral https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3948: *V1* and *V2*, and the result *%vN* must be of type *T*. *T* nust be an must https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4017: Arguments *V1* and *V2*, and the result *%vN* must be of type *T*. *T* nust be must https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4086: type *T*. *T* nust be an integral, or a vector of integrals. *N* is must https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4384: The floatint point remainder instruction returns the remainder of the quotient floating https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4691: fit in in *T2*, then the higer order bits are dropped. higher https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4891: to. Both types must be integral types, or integarl vectors with the same number integral https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5055: The floating point to unsigned integer instruction coverts floating point values converts https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5127: The floating point to signed integer instruction coverts floating point values converts https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5333: contents of the value. The bitsize of the type of the value must be the same as bit size https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5334: the bitsize of the type it is casted to. bit size https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5334: the bitsize of the type it is casted to. casted or cast? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5515: The floating point comparison instruciton compares floating point values and instruction https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5519: elements in *V1* and *V2*. Each comparison always yeilds an i1. If *T* is a yields https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5543: true 15 Alwyas true Always https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5613: existing vectors and generate resulting (new) vectors. This section instroduces introduces https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5654: **Constrants** Constraints https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5893: with the incoming edge from the corresponding predicessor block for which the predecessor https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5913: for the phi node while *%bB1* through *%bBM* are the corresponding predicessor predecessor https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5914: blocks. Each *VI* reaches via the incoming predicessor edge from block *%bBI* predecessor https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5995: The *select* instruction choses pairs of values *V1* and *V2*, based on chooses https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6054: the PNaCl translator is free to perform tail call optimiziation. That is, the optimization https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6083: in the order specified. The type of arugment *AI* must be type *TI* (for all I, argument https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6087: The types of the arugments must match the corresponding types of the function arguments https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6157: in the order specified. The type of arugment *AI* must be type *TI* (for all I, argument https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6161: The types of the arugments must match the corresponding types of the function arguments https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6242: arugment *AI* must be type *TI* (for all I, 1 <= I <= N). Flag *TAIL* is argument https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6332: *A1* through *AM* are passed in the order specified. The type of arugment *AI* argument
Some driveby syntax nits. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... File native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst (right): https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:26: the the abbreviations are used to convert PNaCl records into the sequence of bits. Wrap at 80 here and a few lines down. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:741: **Syntax** Would it be better use a third level of section headers. IIRC using **Foo** is just adding a paragraph with some bold text rather than telling rest that this is a header. You could use ~~~~~~~~~ underlining maybe? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:778: .. naclcode:: If you just want to fixed width font than I prefer not to use 'naclcode'. Just add :: to the end of the preceding paragraph: **Record**:: https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6456: ========== ========================================================================== Can you shorten the ======== lines to be 80 chars since none of the contents is longer than 80? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6535: It is also used to test if two types are either primitive (i.e. UnderlyingCount returns Wrap at 80.
comments from partial scan https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... File native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst (right): https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2: Pnacl Bitcode File Reference Manual PNaCl https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:48: For every kind of record, there is a default method for converting records into nit: is "default" important for this first sentence? vs "For every kind of record, there is a method for converting records into bit sequences"... Abbreviations can be user defined, but there are also pre-defined defaults. Something like that? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:100: of functions within the program. Would it make sense to talk about any subblocks nested within the module block? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:151: association is (again) positional rather than explicit. That is, the Nth After the reordering suggested in the previous rev of this, this is now the first time positional association is mentioned. So the "again" can be removed. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:153: declared) function address record in the module block. I'm not sure it's clear that there are "function address records" in the module block, until this point. Would it make sense to talk about that in the module block description earlier? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:183: values are used. Currently there are four global record codes. To make these Maybe add a link to the later deep dive? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:276: In general, global identifiers are tied to a specific type of block. Local There's some redundancy between this paragraph and the next. Can that be cleaned up? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:345: with the rules, is a notion of :ref:`link_for_global_state_section`. The global For "notion of ... link_for_global_state_section" Should that have a description other than "link_for_global_state_section"? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:363: functions can be found in :ref:`link_for_support_functions_section`. similar, does this "found in :ref:..." end up showing a more readable link text than "found in link_for_support_functions_section" ? Does RST end up pulling the section header in to fill the link text? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:374: upper case alphanumeric characters are named values. if we mix lower and upper "if" -> "If" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:403: constants. Otherwise the record element is a name that is defined by the other At least to me, this sentence is a bit hard to understand: "a name that is defined by the other subsections associated with the construct" mean? Why subsections, plural? What's the difference between name vs identifier (identifier seemed better defined than name)? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:487: version 2, PNaClBitcode files. The first four bytes define the magic number of "PNaClBitcode" is that meant to have a space between? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:513: .. _link_for_memory_blocks_and_alignment_section: This isn't linked to till later, so can it be moved later? Is there a theme/higher level topic to group this under? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:519: memory blocks. Alignment is address placement of these memory blocks. Alignment Is there some word missing in the sentence "Alignment is address placement of these memory blocks"? Are the later sentences enough to describe alignment? (then you can remove that one sentence) https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:532: if the data has an alignment that is larger than 1. As such, chosing a larger "chosing" -> "choosing" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:566: placed at any memory address. they can't be placed at "an arbitrary" memory address (vs "any")? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:588: restrict integral types to i32 and i64). The i32 and i64 parameter/return type restriction doesn't appear till later. Do intrinsic functions need to be described here, or can it be moved till later also? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:594: Can you add something to help w/ transitions between the previous section and the upcoming sections? Seems like this is about to deep dive into the various blocks (or after Global State) https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:622: updates to map *TypeOf* (when given a type ID). Hence, *updates* subsections capitalize "Updates" for consistency https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:638: this association, we use the set: nit: Just a set of IDs doesn't seem sufficient to "capture this association". Seems like there would need to be map, instead, for ID -> Block. It does indicate that a function has an implementation Block though? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:662: The number of parameters defined for a function. So this is the only one that's not "so far". https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:672: function block) so far. "so far" appears twice here https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:731: All special records can't have user-defined abbreviations associated with Are "special records" the same as "global records"? Might be better to stick with "global" than special. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:760: There are no examples for the header record, since it is not part of PNaClAsm. Instead of "since it is not part of", how about "since there is not syntax"? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:904: Fixed(N) 0, 1, N Encode value as a fixed sequence of N bit. "N bit" -> "N bits" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:905: Vbr(N) 0, 2, N Encode value using a variable bit rate of N end with period (for consistency w/ the rest of these) https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:907: Array 0, 3 Allow zero or more of the enclosed encoding Clarify what is the "enclosed encoding"? It's the next element in the <E1, ... EM>? edit: okay, that's described below... In the pnacl-bcdis syntax you use "array(Ei+1)" though. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1163: defines the record I wanted to check if this was meant to be a complete sentence: "defines the record"? This also seems to be the only type w/ two examples instead of one. Is one sufficient? Also, perhaps in the second example, you can describe how "void"'s type ID is referenced by the function type definition? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1382: *Updates* double star the Updates for consistency? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3293: NumBasicBlocks < ExpectedBasicBlocks Do we need to have the NumBasicBlocks < ExpectedBasicBlocks constraint here, if it's not updated? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5148: naclcode ?
Early review, I haven't gotten far at all, but this is pretty hard to dive into. I really think a figure illustrating things, as well as links from one section to another, would be tremendous help in getting started. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... File native_client_sdk/src/doc/_book.yaml (right): https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/_book.yaml:77: path: /native-client/dev/reference/pnacl-bitcode-abi Each of the two above documents should say how they're different. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... File native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst (right): https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:11: Introduction The intro is pretty long. I think having a figure illustrating the three layers would make it much easier to follow the explanation. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:70: records. Mention pnacl-bccompress here? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:85: long*). Code should be in backticks: ``long long`` https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:89: respectively). The file reference/pnacl-c-cpp-language-support.rst has a section on IEEE-754, you should link to it (you'll need to add an anchor in that file for the link to work). https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:127: block. This block appears at the beginning of the module block. Could you make all of the above links to the sections that describe them below. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:186: PNaCl bitcode files do not have record codes >= 2**16. You can do superscripts with: 2\ :sup:`16`\ . (note that spaces need to be escaped after the 2 and before the period) https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:194: The value *v0* is the record code. The remaining values, *v1* through *vN*, are You should use ``code quotes`` here, to be consistent with the naclcode above. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:197: not allowed. *a* is the index to the abbreviation used to convert the record to Same for ``a`` https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:284: the prefix character *'%'*. I'd just use ``code quotes`` here too. Also 5 paragraphs up, when you defined these, I'd be consistent.
https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... File native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst (right): https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:26: the the abbreviations are used to convert PNaCl records into the sequence of bits. the the https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:34: XML, PNaCl records have a notion of tags (i.e. the first element in a record, Instead of "tags", can you say "a tag", to keep singular/plural agreement with the clarification in parentheses? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:44: program, and converts each record into a (variable) sequence of bits. The output "variable" - do you mean "variable-length"? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:120: Each implemented function, that uses constants in its instructions, defines a I would remove both commas. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:121: constant block. Constants blocks appear within the corresponding function "constants block", right? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:130: subsections will dive more deeply into the constraints on how blocks must be Use "sections" instead of "subsections"? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:131: laid out. This section only presents the overall concepts of what kinds of data Either "what kinds of data are stored" or "what kind of data is stored". https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:141: instance can appear. The function and constant blocks define local identifiers, only a single instance of each global identifier? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:177: <http://llvm.org/docs/BitCodeFormat.html>`_). For backwards backward? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:182: kinds of block. To separate global record codes from local record codes, large blocks https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:184: cases clear, and to leave room for lots of future growth in PNaClAsm, these I would remove "lots of", maybe say "to leave ample room". https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:200: While most records (for a given record code) are of the same length, it is not are of --> have ? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:205: the corresponding contruct (associated with the record) to determine the construct https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:236: A block may (in addition), define a list of block specific, user-defined, may (in addition), --> may, in addition, ? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:246: PNaClAsm requires that you specify the number of bits needed to read that you specify --> specifying https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:258: value is given a corresponding unique identifier (i.e. *ID*). In PNaClAsm. each PNaClAsm. --> PNaClAsm, (period to comma) https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:290: that must be (explicitly) passed to the PNaCl translator, and downloaded though though --> through (or "over" or "via") https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:359: processed. The **Examples** subsection gives one (or more) examples of using remove parentheses? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:367: and enclosed in *<* and *>* brackets. These abbreviation are optional in the abbreviations https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:547: In PNaClAsm, the valid load/store alignments are: Do we say anything about <16 x i1> alignment? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:570: Intrinsic functions Functions (capitalized for consistency) https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:587: any integral type for arguments/results, unlike ordinary functions (which The document has mixed use of "integer type" and "integral type". I'm not sure I understand the distinction between the two. If there is none, could we just use "integer" throughout? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:647: Each block defines one (or more) kinds of values. Value indices are generated remove parentheses https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:652: The number of types defined so far (in the types block) period at the end for consistency https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:705: Global records capitalize Records https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1018: integral and floating types. There are a lot of uses of "floating type", and a lot of uses of "floating point type". Is there a difference in meaning? If not, can we use one of them consistently? Preferably "floating point type". https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1041: types are defined by the types block. All remaining records defines a type. The defines --> define https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1340: and an uderlying primitive data type. underlying https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1443: allowed. For intrisic functions, all integral types are allowed for both return and 80-char https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1484: Globals block capitalize Block https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1510: inserts a "}" after the last initializer record associated compound record. This associated *with the* compound record ? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1534: *@g1*. @g0 is 8 bytes long, and initialized to zero. @g1 is with 6 bytes: "1 2 3 g1 is initialized with ? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1893: reloc V + O; <A> I don't have a constructive suggestion here, but "O" looks too much like "0". https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2018: The valuesymtab block ref does not define any values. Its only goal is to Is "ref" supposed to be here? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2124: Each function block defines the corresponding control flow graph for each control flow graph --> intermediate representation This makes it consistent with line 146 above. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2194: executable, All functions should be *defined* and *internal*. The exception to All --> all https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2194: executable, All functions should be *defined* and *internal*. The exception to "exception ... is" or "exceptions ... are" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2196: since intrinsic functions will automatically converted to appropriate code by will be automatically https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2302: Constants blocks define literal constants used within each function. It's intent It's --> Its https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2468: The *integer literal* record creates an integer literal for the integral type *T* 80-col https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2535: Floating point literal Floating Point Literal https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2623: A function implementation contains a list of basic blocks, forming the CFG I don't think you need to use the CFG abbreviation, since it's not used anywhere else in the document. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2630: block. Blocks boundaries are determined by *terminator* instructions. The Block boundaries https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2676: Function enter Function Enter https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2833: The return instruction returns control to the calling function. Do you want "return instruction" or "return void instruction"? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3154: The switch instruction transfer control to a basic block in B0 through BN. transfers https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3273: The result returned is the mathematical result modulo *exp(2,n)*, where *n* is Here (and 2 other places) you use exp(2,n), but in 3 places near the beginning, you use 2**n. Can these be consistent? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3418: In the subtract instruction, integral type i1 (or a vector on integrap type i1) multiply instruction https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3489: negative integer by -1. The behaviour for this case is undefined. Maybe say behavior is "currently" undefined? Since we probably want to fix that. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3623: integral type i1) is disallowed. Division by zero is guaranteed to trap. You need the same comment as sdiv about undefined behavior when dividing min_int by -1. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3753: (statically or dynamically) is negative or equal to or larger than the remove "is" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4341: The float divide instruction returns the quotient of its two floating point divide instruction https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4343: *T*. *T* must be a floating type, or a vector of a floating point type. *N* is floating type --> floating point type https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4443: Memory Creation And Access Instructions don't capitalize "and" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4741: defining the truncated type. Both types must be floating point types, or Can we just say T1 must be double and T2 must be float? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4892: of Elements. The bit size of the source type must be smaller than the bit size Elements --> elements https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4964: Floating point Extending Instruction point --> Point https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4968: extend it to. Both types must be floating types, or vectors of floating a Like above, can we just say T1 must be float and T2 must be double? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5035: Floating Point To Unsigned Integer Instruction To --> to ? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5039: values to an unsigned integers. remobe "an" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5055: The floating point to unsigned integer instruction coverts floating point values converts a floating point value https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5058: integral vector type. If either type is a vector type, they both must be and remove "be and" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5106: Floating Point To Signed Integer Instruction To --> to ? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5127: The floating point to signed integer instruction coverts floating point values converts a floating point value https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5130: integral vector type. If either type is a vector type, they both must be and Remove "be and" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5177: Unsigned Integer To Floating Point Instruction To --> to ? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5197: The unsigned integer to floating point instruction converts unsigned integers to converts an unsigned integer https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5200: vector type. If either type is a vector type, they both must be and have the remove "be and" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5253: Signed Integer To Floating Point Instruction To --> to ? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5273: The signed integer to floating point instruction converts signed integers to its converts a signed integer https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5276: vector type. If either type is a vector type, they both must be and have the remove "be and" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5336: **Sytax** Syntax https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5652: If *I* exceeds the length of *V*, the results is undefined. result https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5738: If *I* exceeds the length of *V*, the results is undefined. result https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5784: Forward type declaration Forward Type Declaration https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5893: with the incoming edge from the corresponding predicessor block for which the for which --> that ? https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6002: be chosen. Otherwise the conrresponding value from *V2* will be chosen. corresponding https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6049: function, is reached control flow continues with instruction after the function Commas more like this? When a *ret* instruction in the called function is reached, control flow ... https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6049: function, is reached control flow continues with instruction after the function with --> with the https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6053: If the *tail* flag is associated with the call instruction, then optimizers in Remove "optimizers in", I think. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6084: 1 <=I <= N). Flag *TAIL* is optional. If it is included, it must the the the the --> be the https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6156: signature returns is not type void. The arguments *A1* through *AM* are passed returns --> returned https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6158: 1 <= I <= N). Flag *TAIL* is optional. If it is included, it must the the the the --> be the https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6243: optional. If it is included, it must the the literal *tail*. the the --> be the https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6334: included, it must the the literal *tail*. the the --> be the https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6436: Bitcode ID's of the forms *@fN*, *@gN*, *%pN*, *%cN*, and *%vN*, are combined ID's --> IDs
Replaced by CL: https://codereview.chromium.org/725333002 https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... File native_client_sdk/src/doc/_book.yaml (right): https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/_book.yaml:77: path: /native-client/dev/reference/pnacl-bitcode-abi On 2014/07/28 20:58:11, JF wrote: > Each of the two above documents should say how they're different. Done. Changed to: 1) Contents of PNaCl bitcode files 2) PNaCl Bitcode Reference Manual https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... File native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst (right): https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2: Pnacl Bitcode File Reference Manual On 2014/07/28 19:20:35, jvoung wrote: > PNaCl Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:11: Introduction On 2014/07/28 20:58:11, JF wrote: > The intro is pretty long. I think having a figure illustrating the three layers > would make it much easier to follow the explanation. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:26: the the abbreviations are used to convert PNaCl records into the sequence of bits. On 2014/07/28 18:28:31, Sam Clegg wrote: > Wrap at 80 here and a few lines down. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:26: the the abbreviations are used to convert PNaCl records into the sequence of bits. On 2014/07/28 22:52:08, stichnot wrote: > the the Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:34: XML, PNaCl records have a notion of tags (i.e. the first element in a record, On 2014/07/28 22:52:03, stichnot wrote: > Instead of "tags", can you say "a tag", to keep singular/plural agreement with > the clarification in parentheses? Rewrote to fix agreement. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:44: program, and converts each record into a (variable) sequence of bits. The output On 2014/07/28 22:52:04, stichnot wrote: > "variable" - do you mean "variable-length"? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:48: For every kind of record, there is a default method for converting records into On 2014/07/28 19:20:36, jvoung wrote: > nit: is "default" important for this first sentence? > > vs > > "For every kind of record, there is a method for converting records into bit > sequences"... Abbreviations can be user defined, but there are also pre-defined > defaults. Something like that? > Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:70: records. On 2014/07/28 20:58:12, JF wrote: > Mention pnacl-bccompress here? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:85: long*). On 2014/07/28 20:58:12, JF wrote: > Code should be in backticks: ``long long`` Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:89: respectively). On 2014/07/28 20:58:11, JF wrote: > The file reference/pnacl-c-cpp-language-support.rst has a section on IEEE-754, > you should link to it (you'll need to add an anchor in that file for the link to > work). Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:100: of functions within the program. On 2014/07/28 19:20:35, jvoung wrote: > Would it make sense to talk about any subblocks nested within the module block? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:120: Each implemented function, that uses constants in its instructions, defines a On 2014/07/28 22:52:04, stichnot wrote: > I would remove both commas. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:121: constant block. Constants blocks appear within the corresponding function On 2014/07/28 22:52:03, stichnot wrote: > "constants block", right? Correct, and fixed. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:127: block. This block appears at the beginning of the module block. On 2014/07/28 20:58:11, JF wrote: > Could you make all of the above links to the sections that describe them below. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:130: subsections will dive more deeply into the constraints on how blocks must be On 2014/07/28 22:52:04, stichnot wrote: > Use "sections" instead of "subsections"? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:131: laid out. This section only presents the overall concepts of what kinds of data On 2014/07/28 22:52:06, stichnot wrote: > Either "what kinds of data are stored" or "what kind of data is stored". Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:141: instance can appear. The function and constant blocks define local identifiers, On 2014/07/28 22:52:04, stichnot wrote: > only a single instance of each global identifier? Yes. Each global is represented by a "global variable" value, and each of these values have a single identifier associated with it. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:151: association is (again) positional rather than explicit. That is, the Nth On 2014/07/28 19:20:35, jvoung wrote: > After the reordering suggested in the previous rev of this, this is now the > first time positional association is mentioned. So the "again" can be removed. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:153: declared) function address record in the module block. On 2014/07/28 19:20:35, jvoung wrote: > I'm not sure it's clear that there are "function address records" in the module > block, until this point. Would it make sense to talk about that in the module > block description earlier? Added a short paragraph before the paragraph on function blocks that describes that only function addresses are defined as values in the module block. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:177: <http://llvm.org/docs/BitCodeFormat.html>`_). For backwards On 2014/07/28 22:52:08, stichnot wrote: > backward? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:182: kinds of block. To separate global record codes from local record codes, large On 2014/07/28 22:52:03, stichnot wrote: > blocks Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:183: values are used. Currently there are four global record codes. To make these On 2014/07/28 19:20:36, jvoung wrote: > Maybe add a link to the later deep dive? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:184: cases clear, and to leave room for lots of future growth in PNaClAsm, these On 2014/07/28 22:52:03, stichnot wrote: > I would remove "lots of", maybe say "to leave ample room". Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:186: PNaCl bitcode files do not have record codes >= 2**16. On 2014/07/28 20:58:11, JF wrote: > You can do superscripts with: > 2\ :sup:`16`\ . > > (note that spaces need to be escaped after the 2 and before the period) Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:194: The value *v0* is the record code. The remaining values, *v1* through *vN*, are On 2014/07/28 20:58:12, JF wrote: > You should use ``code quotes`` here, to be consistent with the naclcode above. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:197: not allowed. *a* is the index to the abbreviation used to convert the record to On 2014/07/28 20:58:11, JF wrote: > Same for ``a`` Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:200: While most records (for a given record code) are of the same length, it is not On 2014/07/28 22:52:08, stichnot wrote: > are of --> have ? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:205: the corresponding contruct (associated with the record) to determine the On 2014/07/28 22:52:06, stichnot wrote: > construct Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:236: A block may (in addition), define a list of block specific, user-defined, On 2014/07/28 22:52:03, stichnot wrote: > may (in addition), --> may, in addition, > ? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:246: PNaClAsm requires that you specify the number of bits needed to read On 2014/07/28 22:52:08, stichnot wrote: > that you specify --> specifying Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:258: value is given a corresponding unique identifier (i.e. *ID*). In PNaClAsm. each On 2014/07/28 22:52:07, stichnot wrote: > PNaClAsm. --> PNaClAsm, > (period to comma) Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:276: In general, global identifiers are tied to a specific type of block. Local On 2014/07/28 19:20:35, jvoung wrote: > There's some redundancy between this paragraph and the next. Can that be cleaned > up? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:284: the prefix character *'%'*. On 2014/07/28 20:58:12, JF wrote: > I'd just use ``code quotes`` here too. Also 5 paragraphs up, when you defined > these, I'd be consistent. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:290: that must be (explicitly) passed to the PNaCl translator, and downloaded though On 2014/07/28 22:52:05, stichnot wrote: > though --> through > (or "over" or "via") Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:291: the internet. On 2014/07/28 18:21:07, stichnot wrote: > Internet? Changed to 'cloud'. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:345: with the rules, is a notion of :ref:`link_for_global_state_section`. The global On 2014/07/28 19:20:35, jvoung wrote: > For "notion of ... link_for_global_state_section" > > Should that have a description other than "link_for_global_state_section"? Good point. Fixing. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:353: terms of data within the globa state and the corresponding syntax. It also On 2014/07/28 18:21:08, stichnot wrote: > global Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:359: processed. The **Examples** subsection gives one (or more) examples of using On 2014/07/28 22:52:09, stichnot wrote: > remove parentheses? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:363: functions can be found in :ref:`link_for_support_functions_section`. On 2014/07/28 19:20:35, jvoung wrote: > similar, does this "found in :ref:..." end up showing a more readable link text > than "found in link_for_support_functions_section" ? Does RST end up pulling the > section header in to fill the link text? Just checked. If you don't specify text with a reference, it uses the corresponding name associated with the link. Hence, that is why I dropped the text on this, and the "global state" links. However, since the title of these sections may change, fixing the :ref:'s to provide an explicit name. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:367: and enclosed in *<* and *>* brackets. These abbreviation are optional in the On 2014/07/28 22:52:09, stichnot wrote: > abbreviations Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:374: upper case alphanumeric characters are named values. if we mix lower and upper On 2014/07/28 19:20:34, jvoung wrote: > "if" -> "If" Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:376: are literal while the upper case sequence of alphanumeric charaters denote rule On 2014/07/28 18:21:08, stichnot wrote: > characters Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:403: constants. Otherwise the record element is a name that is defined by the other On 2014/07/28 19:20:34, jvoung wrote: > At least to me, this sentence is a bit hard to understand: "a name that is > defined by the other subsections associated with the construct" mean? > > Why subsections, plural? What's the difference between name vs identifier > (identifier seemed better defined than name)? Tried cleaning this up (including using identifier). https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:487: version 2, PNaClBitcode files. The first four bytes define the magic number of On 2014/07/28 19:20:34, jvoung wrote: > "PNaClBitcode" is that meant to have a space between? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:492: bitsequences using default abbreviations. On 2014/07/28 18:21:05, stichnot wrote: > bit sequences Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:513: .. _link_for_memory_blocks_and_alignment_section: On 2014/07/28 19:20:35, jvoung wrote: > This isn't linked to till later, so can it be moved later? > > Is there a theme/higher level topic to group this under? I didn't see a better place to put this, but I will think about it some more. Decided (at least for the moment) to move this past the instructions, and before the section on support functions. I also moved the "intrinsic functions" section to appear after instructions. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:519: memory blocks. Alignment is address placement of these memory blocks. Alignment On 2014/07/28 19:20:34, jvoung wrote: > Is there some word missing in the sentence "Alignment is address placement of > these memory blocks"? Are the later sentences enough to describe alignment? > (then you can remove that one sentence) Removing the sentence. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:532: if the data has an alignment that is larger than 1. As such, chosing a larger On 2014/07/28 19:20:35, jvoung wrote: > "chosing" -> "choosing" Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:535: On loads and stores, the aligment in the instruction is used to communicate what On 2014/07/28 18:21:05, stichnot wrote: > alignment Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:539: use that information to potentially chose a more efficent sequence of On 2014/07/28 18:21:05, stichnot wrote: > efficient Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:547: In PNaClAsm, the valid load/store alignments are: On 2014/07/28 22:52:07, stichnot wrote: > Do we say anything about <16 x i1> alignment? Added not applicable entries since they can be loaded/stored. Also extended note below to clarify this. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:566: placed at any memory address. On 2014/07/28 19:20:36, jvoung wrote: > they can't be placed at "an arbitrary" memory address (vs "any")? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:570: Intrinsic functions On 2014/07/28 22:52:03, stichnot wrote: > Functions (capitalized for consistency) Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:586: calling type constraints as ordinary functions. That is, an instrisic can use On 2014/07/28 18:21:08, stichnot wrote: > intrinsic Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:587: any integral type for arguments/results, unlike ordinary functions (which On 2014/07/28 22:52:06, stichnot wrote: > The document has mixed use of "integer type" and "integral type". I'm not sure > I understand the distinction between the two. If there is none, could we just > use "integer" throughout? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:587: any integral type for arguments/results, unlike ordinary functions (which On 2014/07/28 22:52:06, stichnot wrote: > The document has mixed use of "integer type" and "integral type". I'm not sure > I understand the distinction between the two. If there is none, could we just > use "integer" throughout? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:588: restrict integral types to i32 and i64). On 2014/07/28 19:20:34, jvoung wrote: > The i32 and i64 parameter/return type restriction doesn't appear till later. Do > intrinsic functions need to be described here, or can it be moved till later > also? Again, moved to appear after sections on instructions. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:594: On 2014/07/28 19:20:35, jvoung wrote: > Can you add something to help w/ transitions between the previous section and > the upcoming sections? > > Seems like this is about to deep dive into the various blocks (or after Global > State) Added a "road map" section to describe the remaining sections. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:622: updates to map *TypeOf* (when given a type ID). Hence, *updates* subsections On 2014/07/28 19:20:35, jvoung wrote: > capitalize "Updates" for consistency Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:627: represent the function addrress which is a pointer (and pointers are alwyas On 2014/07/28 18:21:05, stichnot wrote: > address Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:627: represent the function addrress which is a pointer (and pointers are alwyas On 2014/07/28 18:21:06, stichnot wrote: > always Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:638: this association, we use the set: On 2014/07/28 19:20:35, jvoung wrote: > nit: Just a set of IDs doesn't seem sufficient to "capture this association". > Seems like there would need to be map, instead, for ID -> Block. > > It does indicate that a function has an implementation Block though? Changed to say: "To indicate which functions have implementations, we use the set:" https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:647: Each block defines one (or more) kinds of values. Value indices are generated On 2014/07/28 22:52:08, stichnot wrote: > remove parentheses Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:652: The number of types defined so far (in the types block) On 2014/07/28 22:52:04, stichnot wrote: > period at the end for consistency Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:662: The number of parameters defined for a function. On 2014/07/28 19:20:34, jvoung wrote: > So this is the only one that's not "so far". Yes. Mainly because we introduce them all at once, from the corresponding type signature of the function. Added note to clarify. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:672: function block) so far. On 2014/07/28 19:20:34, jvoung wrote: > "so far" appears twice here Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:705: Global records On 2014/07/28 22:52:04, stichnot wrote: > capitalize Records Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:731: All special records can't have user-defined abbreviations associated with On 2014/07/28 19:20:35, jvoung wrote: > Are "special records" the same as "global records"? Might be better to stick > with "global" than special. Good point. Missed this when I changed from "special" to "global". https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:741: **Syntax** On 2014/07/28 18:28:31, Sam Clegg wrote: > Would it be better use a third level of section headers. IIRC using **Foo** is > just adding a paragraph with some bold text rather than telling rest that this > is a header. You could use ~~~~~~~~~ underlining maybe? I don't really want to distinguish these. The reasons are: 1) In some cases, we exceed the maximum level for section headers. 2) I don't want these subsections showing up on the sidebar. 3) I want it easy to move these around, without having to edit. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:760: There are no examples for the header record, since it is not part of PNaClAsm. On 2014/07/28 19:20:34, jvoung wrote: > Instead of "since it is not part of", how about "since there is not syntax"? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:778: .. naclcode:: On 2014/07/28 18:28:31, Sam Clegg wrote: > If you just want to fixed width font than I prefer not to use 'naclcode'. Just > add :: to the end of the preceding paragraph: > > **Record**:: Ok. I did not see that in the available documentation. Changing where applicable. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:904: Fixed(N) 0, 1, N Encode value as a fixed sequence of N bit. On 2014/07/28 19:20:35, jvoung wrote: > "N bit" -> "N bits" Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:905: Vbr(N) 0, 2, N Encode value using a variable bit rate of N On 2014/07/28 19:20:35, jvoung wrote: > end with period (for consistency w/ the rest of these) Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:907: Array 0, 3 Allow zero or more of the enclosed encoding On 2014/07/28 19:20:36, jvoung wrote: > Clarify what is the "enclosed encoding"? It's the next element in the <E1, ... > EM>? > > edit: okay, that's described below... > > In the pnacl-bcdis syntax you use "array(Ei+1)" though. I tried to clean this up, but I'm still not sure it is clear. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1010: corresponding type identifer in the types block. Hence, the requirement that the On 2014/07/28 18:21:07, stichnot wrote: > identifier Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1018: integral and floating types. On 2014/07/28 22:52:07, stichnot wrote: > There are a lot of uses of "floating type", and a lot of uses of "floating point > type". Is there a difference in meaning? If not, can we use one of them > consistently? Preferably "floating point type". Changed to use "floating point" everywhere. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1041: types are defined by the types block. All remaining records defines a type. The On 2014/07/28 22:52:04, stichnot wrote: > defines --> define Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1163: defines the record On 2014/07/28 19:20:34, jvoung wrote: > I wanted to check if this was meant to be a complete sentence: "defines the > record"? This also seems to be the only type w/ two examples instead of one. Is > one sufficient? > > Also, perhaps in the second example, you can describe how "void"'s type ID is > referenced by the function type definition? Simplified to a single case. Also removed partial sentence. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1338: types are used when multiple primitve data are operated in parallel using a On 2014/07/28 18:21:08, stichnot wrote: > primitive Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1340: and an uderlying primitive data type. On 2014/07/28 22:52:04, stichnot wrote: > underlying Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1382: *Updates* On 2014/07/28 19:20:36, jvoung wrote: > double star the Updates for consistency? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1443: allowed. For intrisic functions, all integral types are allowed for both return and On 2014/07/28 18:21:06, stichnot wrote: > intrinsic Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1443: allowed. For intrisic functions, all integral types are allowed for both return and On 2014/07/28 22:52:07, stichnot wrote: > 80-char Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1484: Globals block On 2014/07/28 22:52:09, stichnot wrote: > capitalize Block Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1510: inserts a "}" after the last initializer record associated compound record. This On 2014/07/28 22:52:07, stichnot wrote: > associated *with the* compound record ? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1534: *@g1*. @g0 is 8 bytes long, and initialized to zero. @g1 is with 6 bytes: "1 2 3 On 2014/07/28 22:52:06, stichnot wrote: > g1 is initialized with ? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1588: Global Variable Addressses On 2014/07/28 18:21:06, stichnot wrote: > Addresses Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1731: A zerofill initializer record intializes a sequence of bytes, associated with a On 2014/07/28 18:21:06, stichnot wrote: > initializes Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:1893: reloc V + O; <A> On 2014/07/28 22:52:06, stichnot wrote: > I don't have a constructive suggestion here, but "O" looks too much like "0". Changed to use "X". https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2018: The valuesymtab block ref does not define any values. Its only goal is to On 2014/07/28 22:52:04, stichnot wrote: > Is "ref" supposed to be here? Removed. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2044: **Semnatics** On 2014/07/28 18:21:08, stichnot wrote: > Semantics Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2047: sequence of anscii characters *B1* through *BN*. On 2014/07/28 18:21:06, stichnot wrote: > ascii or ASCII Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2124: Each function block defines the corresponding control flow graph for each On 2014/07/28 22:52:07, stichnot wrote: > control flow graph --> intermediate representation > > This makes it consistent with line 146 above. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2196: since intrinsic functions will automatically converted to appropriate code by On 2014/07/28 22:52:04, stichnot wrote: > will be automatically Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2302: Constants blocks define literal constants used within each function. It's intent On 2014/07/28 22:52:07, stichnot wrote: > It's --> Its Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2368: The *set type* record deifnes type *T* to be used to type the (immediately) On 2014/07/28 18:21:05, stichnot wrote: > defines Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2422: The *undefined* lieral record creates an undefined literal constant *%cN* for On 2014/07/28 18:21:05, stichnot wrote: > literal Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2468: The *integer literal* record creates an integer literal for the integral type *T* On 2014/07/28 22:52:05, stichnot wrote: > 80-col Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2494: T == ConsgtantsSetType On 2014/07/28 18:21:06, stichnot wrote: > ConstantsSetType? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2623: A function implementation contains a list of basic blocks, forming the CFG On 2014/07/28 22:52:05, stichnot wrote: > I don't think you need to use the CFG abbreviation, since it's not used anywhere > else in the document. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2630: block. Blocks boundaries are determined by *terminator* instructions. The On 2014/07/28 22:52:08, stichnot wrote: > Block boundaries Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2676: Function enter On 2014/07/28 22:52:08, stichnot wrote: > Function Enter Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:2833: The return instruction returns control to the calling function. On 2014/07/28 22:52:08, stichnot wrote: > Do you want "return instruction" or "return void instruction"? Added void. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3131: places, based on a selector value. It is a generaliation of the conditional On 2014/07/28 18:21:05, stichnot wrote: > generalization Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3152: **Sematics** On 2014/07/28 18:21:05, stichnot wrote: > Semantics Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3154: The switch instruction transfer control to a basic block in B0 through BN. On 2014/07/28 22:52:06, stichnot wrote: > transfers Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3273: The result returned is the mathematical result modulo *exp(2,n)*, where *n* is On 2014/07/28 22:52:04, stichnot wrote: > Here (and 2 other places) you use exp(2,n), but in 3 places near the beginning, > you use 2**n. Can these be consistent? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3293: NumBasicBlocks < ExpectedBasicBlocks On 2014/07/28 19:20:34, jvoung wrote: > Do we need to have the NumBasicBlocks < ExpectedBasicBlocks constraint here, if > it's not updated? Yes. It states that you can't branch to an unknown block. However, added comment earlier in document so that it need not appear on all jump instructions. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3418: In the subtract instruction, integral type i1 (or a vector on integrap type i1) On 2014/07/28 18:21:05, stichnot wrote: > integral Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3489: negative integer by -1. The behaviour for this case is undefined. On 2014/07/28 18:21:08, stichnot wrote: > behavior :) Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3489: negative integer by -1. The behaviour for this case is undefined. On 2014/07/28 22:52:05, stichnot wrote: > Maybe say behavior is "currently" undefined? Since we probably want to fix > that. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3623: integral type i1) is disallowed. Division by zero is guaranteed to trap. On 2014/07/28 22:52:09, stichnot wrote: > You need the same comment as sdiv about undefined behavior when dividing min_int > by -1. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3747: the result *%vN* must be of type *T*. *T* nust be an integral, or a vector of On 2014/07/28 18:21:07, stichnot wrote: > must Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3753: (statically or dynamically) is negative or equal to or larger than the On 2014/07/28 22:52:08, stichnot wrote: > remove "is" Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3815: *V2* and the result *%vN* must be of type *T*. *T* nust be an integral, or a On 2014/07/28 18:21:04, stichnot wrote: > must Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3882: and *V2* and and the result *%vN* must be of type *T*. *T* nust be an integral, On 2014/07/28 18:21:09, stichnot wrote: > must Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3893: integrl type i1) is disallowed. On 2014/07/28 18:21:07, stichnot wrote: > integral Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:3948: *V1* and *V2*, and the result *%vN* must be of type *T*. *T* nust be an On 2014/07/28 18:21:07, stichnot wrote: > must Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4017: Arguments *V1* and *V2*, and the result *%vN* must be of type *T*. *T* nust be On 2014/07/28 18:21:05, stichnot wrote: > must Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4086: type *T*. *T* nust be an integral, or a vector of integrals. *N* is On 2014/07/28 18:21:07, stichnot wrote: > must Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4341: The float divide instruction returns the quotient of its two On 2014/07/28 22:52:06, stichnot wrote: > floating point divide instruction Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4343: *T*. *T* must be a floating type, or a vector of a floating point type. *N* is On 2014/07/28 22:52:06, stichnot wrote: > floating type --> floating point type Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4384: The floatint point remainder instruction returns the remainder of the quotient On 2014/07/28 18:21:07, stichnot wrote: > floating Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4443: Memory Creation And Access Instructions On 2014/07/28 22:52:09, stichnot wrote: > don't capitalize "and" Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4691: fit in in *T2*, then the higer order bits are dropped. On 2014/07/28 18:21:06, stichnot wrote: > higher Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4741: defining the truncated type. Both types must be floating point types, or On 2014/07/28 22:52:06, stichnot wrote: > Can we just say T1 must be double and T2 must be float? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4891: to. Both types must be integral types, or integarl vectors with the same number On 2014/07/28 18:21:09, stichnot wrote: > integral Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4892: of Elements. The bit size of the source type must be smaller than the bit size On 2014/07/28 22:52:03, stichnot wrote: > Elements --> elements Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4964: Floating point Extending Instruction On 2014/07/28 22:52:06, stichnot wrote: > point --> Point Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:4968: extend it to. Both types must be floating types, or vectors of floating a On 2014/07/28 22:52:04, stichnot wrote: > Like above, can we just say T1 must be float and T2 must be double? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5035: Floating Point To Unsigned Integer Instruction On 2014/07/28 22:52:05, stichnot wrote: > To --> to ? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5039: values to an unsigned integers. On 2014/07/28 22:52:05, stichnot wrote: > remobe "an" Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5055: The floating point to unsigned integer instruction coverts floating point values On 2014/07/28 22:52:05, stichnot wrote: > converts a floating point value Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5058: integral vector type. If either type is a vector type, they both must be and On 2014/07/28 22:52:06, stichnot wrote: > remove "be and" Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5106: Floating Point To Signed Integer Instruction On 2014/07/28 22:52:05, stichnot wrote: > To --> to ? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5130: integral vector type. If either type is a vector type, they both must be and On 2014/07/28 22:52:03, stichnot wrote: > Remove "be and" Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5148: On 2014/07/28 19:20:36, jvoung wrote: > naclcode ? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5177: Unsigned Integer To Floating Point Instruction On 2014/07/28 22:52:08, stichnot wrote: > To --> to ? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5197: The unsigned integer to floating point instruction converts unsigned integers to On 2014/07/28 22:52:03, stichnot wrote: > converts an unsigned integer Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5200: vector type. If either type is a vector type, they both must be and have the On 2014/07/28 22:52:05, stichnot wrote: > remove "be and" Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5253: Signed Integer To Floating Point Instruction On 2014/07/28 22:52:05, stichnot wrote: > To --> to ? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5273: The signed integer to floating point instruction converts signed integers to its On 2014/07/28 22:52:07, stichnot wrote: > converts a signed integer Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5276: vector type. If either type is a vector type, they both must be and have the On 2014/07/28 22:52:07, stichnot wrote: > remove "be and" Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5333: contents of the value. The bitsize of the type of the value must be the same as On 2014/07/28 18:21:08, stichnot wrote: > bit size Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5334: the bitsize of the type it is casted to. On 2014/07/28 18:21:08, stichnot wrote: > casted or cast? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5336: **Sytax** On 2014/07/28 22:52:08, stichnot wrote: > Syntax Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5515: The floating point comparison instruciton compares floating point values and On 2014/07/28 18:21:06, stichnot wrote: > instruction Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5519: elements in *V1* and *V2*. Each comparison always yeilds an i1. If *T* is a On 2014/07/28 18:21:06, stichnot wrote: > yields Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5543: true 15 Alwyas true On 2014/07/28 18:21:08, stichnot wrote: > Always Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5613: existing vectors and generate resulting (new) vectors. This section instroduces On 2014/07/28 18:21:07, stichnot wrote: > introduces Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5652: If *I* exceeds the length of *V*, the results is undefined. On 2014/07/28 22:52:03, stichnot wrote: > result Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5654: **Constrants** On 2014/07/28 18:21:07, stichnot wrote: > Constraints Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5738: If *I* exceeds the length of *V*, the results is undefined. On 2014/07/28 22:52:08, stichnot wrote: > result Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5784: Forward type declaration On 2014/07/28 22:52:05, stichnot wrote: > Forward Type Declaration Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5893: with the incoming edge from the corresponding predicessor block for which the On 2014/07/28 18:21:05, stichnot wrote: > predecessor Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5893: with the incoming edge from the corresponding predicessor block for which the On 2014/07/28 18:21:05, stichnot wrote: > predecessor Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5913: for the phi node while *%bB1* through *%bBM* are the corresponding predicessor On 2014/07/28 18:21:08, stichnot wrote: > predecessor Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5914: blocks. Each *VI* reaches via the incoming predicessor edge from block *%bBI* On 2014/07/28 18:21:06, stichnot wrote: > predecessor Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:5995: The *select* instruction choses pairs of values *V1* and *V2*, based on On 2014/07/28 18:21:08, stichnot wrote: > chooses Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6002: be chosen. Otherwise the conrresponding value from *V2* will be chosen. On 2014/07/28 22:52:06, stichnot wrote: > corresponding Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6049: function, is reached control flow continues with instruction after the function On 2014/07/28 22:52:03, stichnot wrote: > Commas more like this? > > When a *ret* instruction in the called function is reached, control flow ... Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6049: function, is reached control flow continues with instruction after the function On 2014/07/28 22:52:08, stichnot wrote: > with --> with the Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6053: If the *tail* flag is associated with the call instruction, then optimizers in On 2014/07/28 22:52:09, stichnot wrote: > Remove "optimizers in", I think. Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6054: the PNaCl translator is free to perform tail call optimiziation. That is, the On 2014/07/28 18:21:06, stichnot wrote: > optimization Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6083: in the order specified. The type of arugment *AI* must be type *TI* (for all I, On 2014/07/28 18:21:07, stichnot wrote: > argument Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6084: 1 <=I <= N). Flag *TAIL* is optional. If it is included, it must the the On 2014/07/28 22:52:06, stichnot wrote: > the the --> be the Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6087: The types of the arugments must match the corresponding types of the function On 2014/07/28 18:21:07, stichnot wrote: > arguments Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6156: signature returns is not type void. The arguments *A1* through *AM* are passed On 2014/07/28 22:52:04, stichnot wrote: > returns --> returned Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6157: in the order specified. The type of arugment *AI* must be type *TI* (for all I, On 2014/07/28 18:21:07, stichnot wrote: > argument Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6158: 1 <= I <= N). Flag *TAIL* is optional. If it is included, it must the the On 2014/07/28 22:52:07, stichnot wrote: > the the --> be the Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6161: The types of the arugments must match the corresponding types of the function On 2014/07/28 18:21:08, stichnot wrote: > arguments Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6242: arugment *AI* must be type *TI* (for all I, 1 <= I <= N). Flag *TAIL* is On 2014/07/28 18:21:06, stichnot wrote: > argument Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6243: optional. If it is included, it must the the literal *tail*. On 2014/07/28 22:52:05, stichnot wrote: > the the --> be the Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6332: *A1* through *AM* are passed in the order specified. The type of arugment *AI* On 2014/07/28 18:21:08, stichnot wrote: > argument Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6334: included, it must the the literal *tail*. On 2014/07/28 22:52:05, stichnot wrote: > the the --> be the Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6436: Bitcode ID's of the forms *@fN*, *@gN*, *%pN*, *%cN*, and *%vN*, are combined On 2014/07/28 22:52:07, stichnot wrote: > ID's --> IDs Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6456: ========== ========================================================================== On 2014/07/28 18:28:31, Sam Clegg wrote: > Can you shorten the ======== lines to be 80 chars since none of the contents is > longer than 80? Done. https://codereview.chromium.org/364463002/diff/20001/native_client_sdk/src/do... native_client_sdk/src/doc/reference/pnacl-bitcode-manual.rst:6535: It is also used to test if two types are either primitive (i.e. UnderlyingCount returns On 2014/07/28 18:28:31, Sam Clegg wrote: > Wrap at 80. Done. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
