[PATCH] D26348: Allow convergent attribute for function arguments

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

Few comments inline



================
Comment at: docs/LangRef.rst:1147
+    In some parallel execution models, there exist operations with one or more
+    arguments that must be uniform across threads. Such arguments are called
+    ``convergent``.
----------------
(Based on what I've heard about GPUs) Should you emphasize that they're executed in "lockstep"?  Or is that not a correct statement?


================
Comment at: docs/LangRef.rst:1158
+
+    For every function F, transformations must ensure that every pair of
+    executions of F that is "compatible" with respect to convergent function
----------------
I think you need to be specific and explicit about how you're relating "executions" between the pre-transform and the post-transform functions.  For instance if the pre-transform function is:

```
void f(int* val dereferenceable(8)) {
  int k = *val
}
```

and the post-transform program is:

```
void f(int* val dereferenceable(8)) {
  int k0 = *(long *)val;
  int k = k & 0xff..ff;
}
```

an execution of the pre-transform program where `k = 400` may not make sense in the context of the post-transform program since you need to produce a 8 byte value for `k0`.



================
Comment at: docs/LangRef.rst:1175
+        if (cond) {
+          Tmp1 = Foo(v [convergent])
+        } else {
----------------
What if `Foo` is a function pointer that we loaded from some place (and thus we don't have a declaration that we can inspect to know for sure if the argument is marked `convergent`)?  This rule would imply we can't do this sinking transform in that case.


================
Comment at: docs/LangRef.rst:1199
+    Calling a function F that contains a call-site CS with convergent function
+    arguments is only defined behavior if the only data-dependencies of the
+    convergent function arguments of CS are convergent parameters of F. If this
----------------
This is a little tricky -- what if the source program is:

```
void f(int convergent i);
void g(int convergent j, int /* normal */ k) {
  f(j - j + k);
}
```

Can we simplify the call site to call `f(k)`?  If so, did we introduce UB in functions that call `g`?


https://reviews.llvm.org/D26348