[PATCH] D17012: Update document about convergent attribute.

[Jingyue Wu via llvm-commits]

jingyue accepted this revision.
jingyue added a reviewer: jingyue.
jingyue added a comment.

Just a reminder if you haven't done that already, double-check how the web page looks like before you commit.


================
Comment at: docs/LangRef.rst:1248
@@ +1247,3 @@
+    additional values.  We call these operations (e.g. the
+    ``llvm.cuda.syncthreads`` intrinsic) ``intrinsically convergent``.
+
----------------
jlebar wrote:
> I'm trying to get across that there exist functions that we will never remove `convergent` from, because
> 
>  a) they actually generate convergent behavior, and
>  b) the only reason you'd call one of these functions is if you want this behavior.
> 
> These "intrinsically convergent" functions are different than e.g. foo in
> 
>   void foo() {
>     do_stuff();
>     __syncthreads();
>   }
> 
> foo() generates convergent behavior, but it may or may not be the case that the only reason you'd call foo() is if you want this behavior.  This patch says, we'll preserve the convergent behavior of foo if foo is marked as convergent and it may transitively invoke an intrinsically convergent function.  If the latter isn't true, there's no convergent behavior to preserve, so we can ignore the attribute.
> 
> It sounds like this isn't clear from the language here -- if you can help me with where you stumbled, I can try to rephrase.
Italicizing a definition is more common than monospacing it, so suggest //intrinsically convergent// instead of `intrinsically convergent`. 


================
Comment at: docs/LangRef.rst:1255
@@ -1246,1 +1254,3 @@
+    values.
+
 ``inaccessiblememonly``
----------------
Nit: I don't know how this empty line affects the web display, but I didn't see other attributes (such as `ssq`) have an empty line at the end. 


http://reviews.llvm.org/D17012