[PATCH] Document HLE metadata

[darkbuck]

  Hi Jeffrey

  I did try to and will merge your comments.


================
Comment at: docs/LangRef.rst:1298
@@ +1297,3 @@
+
+.. admonition:: Rationale
+
----------------
Jeffrey Yasskin wrote:
> You've provided an overview here, not a rationale. A rationale would address why HLE hints are important, not just what they do.
I am little confused here. The importantness of HLE hints is that we could leverage hardware HLE support by simply hinting atomic insn. Is that the rationale to introduce HLE hint instead introduce bunch of new atomic primitives?

================
Comment at: docs/LangRef.rst:1301
@@ +1300,3 @@
+  As lock primitives are built upon atomic instructions, HLE hints attached on
+  atomic instructions allow programmers to convert a locked-based critical
+  section into a transaction region to take advantage of hardware transactional
----------------
Jeffrey Yasskin wrote:
> s/locked-based/lock-based/
> 
> Do you happen to have a native English speaker you could get to proofread your LangRef patch before you send out iterations? I don't mind correcting grammar when the meaning's obvious, but there are a couple places I'm having to guess at your meaning, and this review might go faster if I didn't have to.
as non-native speaker, I will try best, :)

================
Comment at: docs/LangRef.rst:1309
@@ +1308,3 @@
+Atomic instructions (:ref:`cmpxchg <i_cmpxchg>`, :ref:`atomicrmw <i_atomicrmw>`,
+and :ref:`atomic store <i_store>`) may take an optional hint of Hardware Lock
+Elision (Speculative Lock Elision or Transactional Lock Elision) to start an
----------------
Jeffrey Yasskin wrote:
> "an optional hint of Hardware Lock Elision" still uses HLE as a magic word. I want you to write this section at an audience who hasn't worked at Intel, which means they don't already know what HLE means.
I did list non-Intel but mostly equivalent terms like SLE (speculative lock elison), TLE (transactional lock elision). If you want to put an introduction of them, we'd better provide a reference link.

================
Comment at: docs/LangRef.rst:1311
@@ +1310,3 @@
+Elision (Speculative Lock Elision or Transactional Lock Elision) to start an
+HLEd critical section by hinting an atomic instruction with HLE *acquire* hint
+and to end an end an HLEd critical section by hinting an atomic instruction
----------------
Jeffrey Yasskin wrote:
> Nit: Here I would probably write out the metadata IR representation. That is, "by hinting an atomic instruction with !hle.lock !{metadata !"acquire"}". That avoids saying "hint" twice.
ok, let's put them like "by annotating .. with ... hint" to avoid put HLE hint details here.

================
Comment at: docs/LangRef.rst:1315
@@ +1314,3 @@
+
+With an HLE *acquire* hint, an atomic instruction is executed speculatively on
+the specified atomic object. It also start the transactional execution until
----------------
Jeffrey Yasskin wrote:
> What does it mean to execute an atomic instruction speculatively?
> 
> I suspect HLE actually *omits* the atomic instruction speculatively, on the assumption that it will be set back to its original value before any other thread could see the change. Please be precise.
sure, speculation here just means the write to that atomic object is omitted, or other behaviour is still the same, e.g. the current value of atomic object will be returned.

================
Comment at: docs/LangRef.rst:1316
@@ +1315,3 @@
+With an HLE *acquire* hint, an atomic instruction is executed speculatively on
+the specified atomic object. It also start the transactional execution until
+a matching atomic instruction with an HLE *release* hint.
----------------
Jeffrey Yasskin wrote:
> I think you mean "It also starts transactional execution, which continues until a matching atomic instruction with an HLE *release* hint."
> 
> However, I believe this sentence is incorrect: the transaction may stop before the instruction with the release hint (a syscall or IO operation would do it, beyond the obvious case of cache line invalidation, right?). Please describe the cases where the transaction might abort, or describe the meaning of the acquire and release hints without referring to an extra "transactional" state.
> 
> Also, what does "matching" mean?
the transactional execution (vs. normal execution) itself means the code may roll back due to abort.

Do people not know "matching" if they use lock to synchronize threads?

================
Comment at: docs/LangRef.rst:1322
@@ +1321,3 @@
+It tries to commit the changes to shared memory, which is not global visible
+before a sucessful commit. If the commit fails, the execution will be restarted
+from the point where an early atomic instruction hinted with HLE *acquire*.
----------------
Jeffrey Yasskin wrote:
> Jeffrey Yasskin wrote:
> > spelling: s/sucessful/successful/
> Why would a commit fail? For example, in [Intel® Architecture Instruction Set Extensions Programming Reference] 8.3.3, it looks like if the release atomic sets the variable to a value other than what it had before the acquire, the commit might fail.
do you mean the commit failing at the point of *release*? 8.3.3 only gives one possible reason (specific to x86 HLE impl). there're other reasons (on x86 or non-x86 targets) that a transaction may fails, e.g. lack of resources.

================
Comment at: docs/LangRef.rst:1325
@@ +1324,3 @@
+
+It's expected that HLE hints are paired similar to lock/unlock usage in thread
+synchronization. However, improper use of HLE hints will not cause functional
----------------
Jeffrey Yasskin wrote:
> grammar: s/similar/similarly/
> 
> What does "similar" actually mean here? Can the hints be paired like a recursive mutex? What about like a reader/writer mutex?
it could be nested but not recursive (personally, recurisve mutex is evil and encourags bad concurrency practice). read/writer lock is OK to be hinted. the only tricky case I am aware of is the ticket lock.


http://llvm-reviews.chandlerc.com/D475