Johannes Doerfert via llvm-dev
2020-Sep-07 22:38 UTC
[llvm-dev] [RFC] Introducing the maynotprogress IR attribute
On 9/7/20 4:48 PM, Nicolai Hähnle wrote:> Hi Johannes, > > > On Mon, Sep 7, 2020 at 11:17 PM Johannes Doerfert > <johannesdoerfert at gmail.com> wrote: >> >> > As a separate comment, I don't find the reference to the C++ spec in >> >> > https://reviews.llvm.org/D86233 to be informative enough. Whenever >> >> > that section of the C++ spec talks about "progress" it is >> referring to >> >> > how some abstract scheduler schedules execution of multiple threads. >> >> > That is, it is talking about the dynamic behavior of the >> >> > implementation. On the other hand, the proposed attribute presumably >> >> > makes a statement about the program itself _assuming that_ its >> >> > execution gets scheduled. So there is no applicable definition of >> >> > "progress" in the cited section of the C++ spec. >> >> >> >> I don't understand. What is the alternative to "assuming that its >> >> execution gets scheduled"? >> > >> > The alternative is dealing with the possibility that execution of a >> > thread is _not_ scheduled by the abstract machine :) >> >> I guess if you do not schedule something there is not much to say about >> it. We usually talk about about all executions and if you don't have >> any, everything is true anyway. Unsure how you see that tie in here >> though. > The point is purely one about understandability of the proposed > LangRef change. The LangRef change talks about "progress" as defined > in [intro.progress] of the C++ spec, but the vast majority of that > section, and _all_ parts of that section that actually use the word > "progress", talk about the kind of forward progress guarantees that > have nothing at all to do with what you're trying to get at in the > proposal. My understanding is that for your proposal here, you're only > really interested in what's written in the very first clause of that > C++ spec section, but that's totally non-obvious from what the > proposed patch to the LangRef says. The point is that the proposed > language is misleading to somebody who approaches it without > preconceptions.I guess we can say something like: [6, see the first section under the title "forward progress"] if you think that makes it easier to read. Would that address your concern or did I still not understand? ~ Johannes> Cheers, > Nicolai > > > > >> ~ Johannes >> >> > This is a serious topic on GPUs, where you may have no or very weak >> > forward progress guarantees, e.g. because threads mapped to the same >> > vector/wave might not make progress while the vector is executing some >> > other divergent part of the CFG, or because more threads are requested >> > than physically fit on the machine. >> > >> > Cheers, >> > Nicolai >> > >> > >> > >> >> >> >> ~ Johannes >> >> >> >> >> >> > Cheers, >> >> > Nicolai >> >> > >> >> > >> >> >> >> [1] https://bugs.llvm.org/show_bug.cgi?id=965 >> >> >> >> [2] >> https://lists.llvm.org/pipermail/llvm-dev/2015-July/088103.html >> >> >> >> [3] >> >> https://lists.llvm.org/pipermail/llvm-dev/2017-October/118558.html >> >> >> >> [4] https://reviews.llvm.org/D38336 >> >> >> >> [5] https://reviews.llvm.org/D65718 >> >> >> >> [6] https://eel.is/c++draft/intro.progress >> >> > >> >> > >> >> > >> >> >> > >> > >> >
Nicolai Hähnle via llvm-dev
2020-Sep-08 13:52 UTC
[llvm-dev] [RFC] Introducing the maynotprogress IR attribute
On Tue, Sep 8, 2020 at 12:40 AM Johannes Doerfert <johannesdoerfert at gmail.com> wrote:> > > On 9/7/20 4:48 PM, Nicolai Hähnle wrote: > > Hi Johannes, > > > > > > On Mon, Sep 7, 2020 at 11:17 PM Johannes Doerfert > > <johannesdoerfert at gmail.com> wrote: > >> >> > As a separate comment, I don't find the reference to the C++ spec in > >> >> > https://reviews.llvm.org/D86233 to be informative enough. Whenever > >> >> > that section of the C++ spec talks about "progress" it is > >> referring to > >> >> > how some abstract scheduler schedules execution of multiple threads. > >> >> > That is, it is talking about the dynamic behavior of the > >> >> > implementation. On the other hand, the proposed attribute presumably > >> >> > makes a statement about the program itself _assuming that_ its > >> >> > execution gets scheduled. So there is no applicable definition of > >> >> > "progress" in the cited section of the C++ spec. > >> >> > >> >> I don't understand. What is the alternative to "assuming that its > >> >> execution gets scheduled"? > >> > > >> > The alternative is dealing with the possibility that execution of a > >> > thread is _not_ scheduled by the abstract machine :) > >> > >> I guess if you do not schedule something there is not much to say about > >> it. We usually talk about about all executions and if you don't have > >> any, everything is true anyway. Unsure how you see that tie in here > >> though. > > The point is purely one about understandability of the proposed > > LangRef change. The LangRef change talks about "progress" as defined > > in [intro.progress] of the C++ spec, but the vast majority of that > > section, and _all_ parts of that section that actually use the word > > "progress", talk about the kind of forward progress guarantees that > > have nothing at all to do with what you're trying to get at in the > > proposal. My understanding is that for your proposal here, you're only > > really interested in what's written in the very first clause of that > > C++ spec section, but that's totally non-obvious from what the > > proposed patch to the LangRef says. The point is that the proposed > > language is misleading to somebody who approaches it without > > preconceptions. > > I guess we can say something like: > [6, see the first section under the title "forward progress"] > if you think that makes it easier to read. > > Would that address your concern or did I still not understand?That would be a step in the right direction, yes. How about the following: "This attribute indicates that the function is permitted to not return or interact with the environment. Functions without this attribute are implicitly ``mustprogress`` and they must eventually return or interact with the environment in some way, e.g. via a side effect or synchronization. ``mustprogress`` is intended to model the requirements of the first section of [intro.progress] of the C++ standard [6]." This avoids using the expression "make progress" in a sense that differs from how the C++ standard defines it. Cheers, Nicolai -- Lerne, wie die Welt wirklich ist, aber vergiss niemals, wie sie sein sollte.
Johannes Doerfert via llvm-dev
2020-Sep-08 14:25 UTC
[llvm-dev] [RFC] Introducing the maynotprogress IR attribute
On 9/8/20 8:52 AM, Nicolai Hähnle wrote:> On Tue, Sep 8, 2020 at 12:40 AM Johannes Doerfert > <johannesdoerfert at gmail.com> wrote: >> >> On 9/7/20 4:48 PM, Nicolai Hähnle wrote: >>> Hi Johannes, >>> >>> >>> On Mon, Sep 7, 2020 at 11:17 PM Johannes Doerfert >>> <johannesdoerfert at gmail.com> wrote: >>>> >> > As a separate comment, I don't find the reference to the C++ spec in >>>> >> > https://reviews.llvm.org/D86233 to be informative enough. Whenever >>>> >> > that section of the C++ spec talks about "progress" it is >>>> referring to >>>> >> > how some abstract scheduler schedules execution of multiple threads. >>>> >> > That is, it is talking about the dynamic behavior of the >>>> >> > implementation. On the other hand, the proposed attribute presumably >>>> >> > makes a statement about the program itself _assuming that_ its >>>> >> > execution gets scheduled. So there is no applicable definition of >>>> >> > "progress" in the cited section of the C++ spec. >>>> >> >>>> >> I don't understand. What is the alternative to "assuming that its >>>> >> execution gets scheduled"? >>>> > >>>> > The alternative is dealing with the possibility that execution of a >>>> > thread is _not_ scheduled by the abstract machine :) >>>> >>>> I guess if you do not schedule something there is not much to say about >>>> it. We usually talk about about all executions and if you don't have >>>> any, everything is true anyway. Unsure how you see that tie in here >>>> though. >>> The point is purely one about understandability of the proposed >>> LangRef change. The LangRef change talks about "progress" as defined >>> in [intro.progress] of the C++ spec, but the vast majority of that >>> section, and _all_ parts of that section that actually use the word >>> "progress", talk about the kind of forward progress guarantees that >>> have nothing at all to do with what you're trying to get at in the >>> proposal. My understanding is that for your proposal here, you're only >>> really interested in what's written in the very first clause of that >>> C++ spec section, but that's totally non-obvious from what the >>> proposed patch to the LangRef says. The point is that the proposed >>> language is misleading to somebody who approaches it without >>> preconceptions. >> I guess we can say something like: >> [6, see the first section under the title "forward progress"] >> if you think that makes it easier to read. >> >> Would that address your concern or did I still not understand? > That would be a step in the right direction, yes. > > How about the following: > > "This attribute indicates that the function is permitted to not return > or interact with the environment. Functions without this attribute are > implicitly ``mustprogress`` and they must eventually return or > interact with the environment in some way, e.g. via a side effect or > synchronization. ``mustprogress`` is intended to model the > requirements of the first section of [intro.progress] of the C++ > standard [6]." > > This avoids using the expression "make progress" in a sense that > differs from how the C++ standard defines it.That sound pretty good to me. ~ Johannes> > Cheers, > Nicolai > > > > -- > Lerne, wie die Welt wirklich ist, > aber vergiss niemals, wie sie sein sollte.