Update Reference “opencilk-language-specification”

[behoppe]

Automatically generated by Netlify CMS

[netlify]

✅ Deploy Preview for sage-licorice-6da44d ready!

Name	Link
🔨 Latest commit	b7b5772
🔍 Latest deploy log	https://app.netlify.com/sites/sage-licorice-6da44d/deploys/630d345e2945080009fb2cef
😎 Deploy Preview	https://deploy-preview-95--sage-licorice-6da44d.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[behoppe]

Hello @dcurtisatmit do you get notified when I comment on your pull request?

[behoppe]

Hi @dcurtisatmit I have implemented a way to display attribution to Intel and Cilk Arts. This article should credit them, so I went in myself to "flick the switch" in the CMS editor ("provide attribution to Intel and Cilk Arts"). It's also editable manually with attribution: true in the front matter. The attribution still won't actually display until this PR is merged with the main branch. (A little more info is at #62.) Thank you.

[neboat]

It's taking me a while to review the entire document and take notes about issues. Here are some of my initial notes.

Introduction

• The date and version number don't make sense to me.
• There are multiple references to a document on the OpenCilk Application Binary Interface, which does not exist. The document includes notes to add links to that document that are not appropriate for the published version of the document.
• The listed related documents either don't exist or are out of date.
• "Together, these documents provide the detail needed to implement a compliant compiler." I'm not convinced that's true. What exactly is that compiler expected to do? Is it expected to implement the correct ABI? Is it expected to consume the OpenCilk runtime's bitcode ABI? None of those issues are discussed here.
  • Is this even a use case for the document that we want to consider at this time? If it's not and we still want to publish this document, we should remove this statement.
• The document references "SIMD loops" and "SIMD-enabled functions," but doesn't define them.
• "serialization" -> "serial projection" (This change is discussed more later.)
• "execution...is in general a directed acyclic graph." It's more precise to say the execution is modeled using a directed acyclic graph.
• Document mentions a "data race." Should this be "determinacy race"?
  • Description of a data race is consistent with determinacy race, not data race.
• "The word "shall"...on a Cilk Plus program." This sentence should refer to an OpenCilk program.

Grammar

• The document makes reference to "jump-statement," "postfix-expression," etc., but it does not define them. Are these terms derived from something else, like the C/C++ language specification? The document should make it clear where these terms come from.
  • In general, the document should make clear how it's defining the grammar.
• The document only describes cilk_spawn before a function call. OpenCilk supports cilk_spawn before other expressions as well. This change should be discussed both in the Grammar and Semantics sections.
  • I don't believe the comment about cilk_spawn for overloaded operators is true of OpenCilk.
• "The rank of a spawned function shall be zero." The document does not define "rank."
  • The link after this sentence to "The section expression" does not work.
• The grainsize-pragma description is incorrect:
  • OpenCilk only allows a constant value in place of "expression." It does not allow arbitrary expressions.
  • OpenCilk does not require the = sign in the grainsize pragma. In other words, the = is optional.
• I don't think the grammar for cilk_for is correct. In particular, I believe the initialization of the loop must be a declaration.
• The formatting of the descripiton of cilk_scope does not match the formatting for the other constructs.
  • The text describes the semantics of cilk_scope in the section that only describes the grammar rules. Based on the organization of the document, discussion of the semantics of cilk_scope should be deferred to later.

Semantics (not complete)

• "...form a directed acyclic graph (DAG) in which spawn points and sync points comprise the vertices..." This description doesn't fit with the earlier description of strands. If a single strand is subdivided into a sequence of shorter strands, then there must be points other than spawn and sync points between those strands. What are those points?
• "Serialization rule": We now call it the "serial projection," not the "serialization." This change needs to be propagated throughout the document.
• The document defines terms "left," "leftmost," etc., but does not use them.
• The semantics of some Cilk keywords means that the serial projection is not simply the result of replacing/eliding the Cilk keyword as the document describes. For example, the evaluation of an assignment spawn or initializer spawn is not technically consistent with the serial projection of that spawn, according to the C++17 standard.
• Although OpenCilk accepts the syntax for cilk_for loops described in the document, OpenCilk admits a wider range of syntax for cilk_for loops than that. Hence, the description of those rules as strict requirements is not technically correct. I discussed some of these issues with Alex for the article he's working on about cilk_for. As I understand it, Alex is working on an updated description of what syntax is valid for cilk_for loops.
  • I suspect there is a simpler and clearer way to describe the cilk_for syntactic constraints, based on how the C++ language specification describes range-for loops.
• "The initialization, condition, and increment parts of a cilk_for shall meet all of the semantic requirements of the corresponding serial for statement." Given how the for statement has evolved in recent versions of C++, I'm not sure this statement is true. Or perhaps it's just not clear what exactly is meant by "the corresponding serial for statement." Presumably, for example, this statement is not meant to include range-for statements in C++.
• Code is not formatted consistently in the description of cilk_for.
• "odr-use" is not defined in this document.

[VoxSciurorum]

I would remove the word "specification". Our goal now is not to write a spec that would allow a clean room implementation of OpenCilk. So rename the document and remove the first paragraph and bulleted list of the introduction.

We don't have any special SIMD features beyond what LLVM provides so remove the first three sentences of the paragraph beginning "An implementation".

Remove the last sentence of the introduction explaining the word "shall". Change the uses of "shall" later in the document from "foo shall be bar" to "foo is bar".

Remove the "Related documents" section.

In "Keywords for Tasking" add the cilk_reducer keyword. Remove the last sentence, which invokes the language of the C standard.

Rewrite the definition of cilk_sync: "The keyword cilk_sync is a complete statement body to be followed by a semicolon."

Rewrite cilk_spawn and cilk_for however TB thinks they work now.

cilk_scope can be defined with text. "The keyword cilk_scope is followed by a block statement (enclosed in curly braces). There is an implicit cilk_sync at the end of the block, before destructors are run." (TB, is that last part true?)

TB says the serialization rule is no longer true in C++ when you spawn an assignment with a side effect on the left hand side.

In the "Spawn" section the examples in red show as x\\\\\\\[g()] where x[g()] was intended.

Is the "Programmer note" under "Sync" still true in OpenCilk?