[PATCH] D48240: Try again to implement a FIFO task queue
Pavel Labath via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 22 03:51:35 PDT 2018
labath added a comment.
I think I noticed one more bug in the !threads implementation. Other than that, this looks fine to me, but I'll leave the final ok to Chandler.
================
Comment at: llvm/include/llvm/Support/TaskQueue.h:144-150
+ auto WrappedTask = [this, Id] {
+ assert(!Tasks.empty());
+ do {
+ Tasks.front()();
+ Tasks.pop_front();
+ } while (Id != NextRunId++);
+ };
----------------
I just realized this could be wrong when the futures are waited on out of order. E.g. if you have F1 and F2, `F2.wait()` will increment `NextRunId` to 2. Then if you do `F1.wait()`, it will loop until NextRunId wraps back to 1 and crash in the process.
Fortunately, with RunIds it should be easy to fix this by changing `!=` to `<`. If we're worried about uint64_t wrapping we can do something like `Id-NextRunId < UINT64_MAX/2`
(Also, this code could be moved into a separate function to decrease the number of instantiations of the lambda.)
https://reviews.llvm.org/D48240
More information about the llvm-commits
mailing list