[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