[PATCH] D26348: Allow convergent attribute for function arguments

[Nicolai Hähnle via llvm-commits]

nhaehnle added inline comments.


================
Comment at: docs/LangRef.rst:1163
+
+    Two runs r1 and r2 of a program are compatible (wrt convergent function
+    attributes) if for every call site CS with a convergent argument, the
----------------
hfinkel wrote:
> nhaehnle wrote:
> > hfinkel wrote:
> > > Please write out "with respect to." Also, you mean convergent function-parameter attributes, not function attributes.
> > > 
> > > The runs of the program terminology I find bothersome, in part because that's not what we're talking about. We're talking about the behavior of different threads within the same program. I think it would be better to say, "Two executions of the calling function, e1 and e2, are compatible..." Then we can say that transformations must preserve compatibility w.r.t. convergent function-parameter attributes for all potential executions of the calling function. We might even further specify that this is for potential simultaneous executions.
> > > 
> > > Finally, I'd really like it if you would include the example we've been discussing in the reference as a non-normative example. I did not really understand the definition until I read @nhaehnle's explanation above.
> > I'm only one person :)
> > 
> > I'm making the editing changes and adding the example.
> > 
> > s/run/execution/ is fine with me, and I'm adding that as an alternative below, but whenever I try to formulate some text that talks about executing a calling function rather than executing a function, I feel confused about the calling vs. the callee function.
> > 
> > Perhaps you can clarify:
> > 
> > > The runs of the program terminology I find bothersome, in part because that's not what we're talking about. We're talking about the behavior of different threads within the same program.
> > 
> > What's the difference between threads and executions in this context?
> > but whenever I try to formulate some text that talks about executing a calling function rather than executing a function, I feel confused about the calling vs. the callee function.
> 
> Feel free to establish some terminology early in the definition for later use. We're talking about a function-parameter attribute. That attribute must appear on the function parameters of some call site. That call site exists in some function (the caller a.k.a. the calling function) and calls some other function (the callee function).
> 
> > What's the difference between threads and executions in this context?
> 
> As I think about it, a thread is some entity that executes things (e.g. functions). Threads execute functions, and each time a thread executes some function, that constitutes an execution of that function.
> 
I see. I played around with this some more, and here's an example I'd like to hear your thoughts on:

  define void @foo(i32 %a) {
    ...
    call void @bar(i32 %a)
    ...
  }

  define void @bar(i32 %a) {
    call void @baz(i32 convergent %a)
  }

This code has a potential problem because at least the parameter of bar should arguably have been labelled convergent.

I'd say that inlining shouldn't have to worry about the convergent attribute at all. However, if the definition we're discussing here takes a function-centric view, then we cannot prove in a target-independent way that inlining bar is correct (because foo gets a new callsite with a convergent function argument, where previously there were no such callsites, i.e. //every// pair of runs of foo was compatible). If the definition takes a program-centric view (i.e., executions that start at foo implicitly also consider the sequence of arguments passed into baz inside bar), then inlining bar is clearly correct.

That would be an argument in favour of staying with the language about "executions of a program" rather than "executions of a function".


================
Comment at: docs/LangRef.rst:1153
+    "whether and when each call site to this function is reached must be
+    uniform across multiple threads", the ``convergent`` attribute on function
+    arguments can be thought of as "the value of this argument at each
----------------
mehdi_amini wrote:
> hfinkel wrote:
> > jlebar wrote:
> > > > While the ``convergent`` attribute on functions can be thought of as "whether and when each call site to this function is reached must be uniform across multiple threads",
> > > 
> > > I know this isn't meant to be normative, but I'm a little concerned by this language.  Specifically, our semantics say that the compiler is not allowed to *introduce* new sources of divergence to convergent calls, but it is also not allowed to *assume* that convergent calls are uniform.
> > > 
> > > I presume the same is true for convergent args -- the compiler cannot introduce divergence into the arg values, but it also cannot assume anything about the arg values as a consequence of their being marked convergent.
> > > 
> > > I know rewriting this to be precise might defeat the purpose of your summary here.  But could we add some weasel words so that people can't point to this as justification that divergent calls / args marked with `convergent` are LLVM UB?
> > This is a good point. We might want to say something like that there is some implementation-defined set of simultaneously-executing threads for which the restriction holds. Thus we won't be able to make an target-independent assumptions about the implied behavior.
> I don't see why we should mention anything about thread or concurrency in the LangRef?
> Also, I'm not thrilled by this paragraph either (to begin with, the sentence about the convergent attribute on function isn't clear to me).
Hmm. I wonder what the general opinion is on whether this paragraph is still helpful at all. The way I see it, it can serve two purposes:

  # Explain the attribute in an intuitive way to someone with an SIMT/SPMD background.
  # Explain why the same term is used here for a function-parameter attribute and elsewhere as a function attribute.

But if it just adds confusion for others, maybe it's best to remove it rather than discuss it to death? After all, the first purpose is kind of fulfilled already by the paragraph before that...

If the paragraph is kept around, then I agree with something like:

> ... whether and when each call site to this function is reached must be uniform across a target-dependent set of simultaneously-executing threads ...


================
Comment at: docs/LangRef.rst:1159
+    Program transformations must ensure that every pair of runs that is
+    compatible with respect to convergent function attributes in the original
+    program must also be compatible in the transformed program. Compatibility
----------------
jlebar wrote:
> Nit, suggest putting "compatible" in quotes so it's clear it's a term we're about to define.
Sure, no problem.


https://reviews.llvm.org/D26348