[PATCH] D26348: Allow convergent attribute for function arguments

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.


================
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 {
----------------
nhaehnle wrote:
> jlebar wrote:
> > 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?
> I'd be happy with it. When I posted something like this on the mailing list initially, Mehdi asked for clarification :)
My concern on the mailing was the original definition was based on "uniformity".

Anyway, what about this example:

```
if (cond) {
  Tmp1 = ...
  Foo(Tmp1 [convergent])
} else {
  Tmp2 = ...
  Foo(Tmp2 [convergent])
}
```

I think your definition does not prevent:

```
if (cond) {
  Tmp1 = ...
} else {
  Tmp2 = ...
}

Tmp3 = phi [Tmp1, Tmp2]

if (cond) {
  Foo(Tmp3 [convergent])
} else {
  Foo(Tmp3 [convergent])
}
```

However it breaks uniformity.


https://reviews.llvm.org/D26348