[PATCH] D26348: Allow convergent attribute for function arguments

[Justin Lebar via llvm-commits]

jlebar added a comment.

I looked at just the langref and intrinsics.td.



================
Comment at: docs/LangRef.rst:1154
+
+    So where the ``convergent`` attribute on functions can be thought of as
+    "whether and when each call site to this function is reached must be
----------------
Nit, I think you want "whereas" or "while".  Sort of like "although" but a less strong sense of contrast.


================
Comment at: docs/LangRef.rst:1168
+    through the transformed program (with the same input / initial conditions)
+    must also be compatible.
+
----------------
This is going to be the paragraph to wordsmith.

I think the approach of saying "A correct transformation has the following property: For any two runs of the program r1 and r2, if r1 and r2 were compatible before the transformation, they must be compatible after the transformation" makes some sense as a way to import SPMD semantics into the langref.

But I am not sure about the definition of 'compatible'.  For one thing, this doesn't work if the transformation adds or removes a callsite with convergent parameters.  But you're sometimes allowed to do this.  For example, you could unroll a loop which contains a convergent-parameter call.

This is really tricky...


================
Comment at: include/llvm/IR/Intrinsics.td:54
+// intrinsic may not be modified in a way that makes the argument value
+// control-dependent on additional values.
+class Convergent<int argNo> : IntrinsicProperty {
----------------
This definition actually seems fairly clear to me, if we s/control-//.  Is there a reason we don't want to use this as our canonical definition?


https://reviews.llvm.org/D26348