[PATCH] D65718: [LangRef] Document forward-progress requirement

JF Bastien via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 9 20:56:49 PDT 2019


jfb added a comment.

In D65718#1624143 <https://reviews.llvm.org/D65718#1624143>, @reames wrote:

> Just a note of caution: I see discussion starting on cornercases.  I would strongly suggest *not* blocking this patch on getting a fully consistent definition.  Having something documented which is "mostly right" is better than no documentation at all.  (Adding a explicit marker of subtleties under discussion is good.)  It's really tempting as reviewers to keep looking for the subtle problems, but remember, the confusion which keeps arising is over the basic need for a side effect, not the exact definition thereof!


I have to disagree strongly. C++ has a forward progress definition and it sounds like we’re codifying something detected from it for llvm IR.

Now say I have code with:

book ready;
// ...
while (!ready)

  __atomic_fence(RELAXED);

Does this need a side effect? Per the current definition, no. Per C++, yes.

So codifying something “mostly ok” is a terrible idea because any further change will affect semantics we’re promised people.

Right now we don’t promise anything. It’s bad! Let’s fix it. But let’s fix it for real, not a band-aid.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65718/new/

https://reviews.llvm.org/D65718





More information about the llvm-commits mailing list