martong added inline comments.
================
Comment at: clang/lib/Analysis/LiveVariables.cpp:522
- // FIXME: we should enqueue using post order.
- for (const CFGBlock *B : cfg->nodes()) {
+ for (const CFGBlock *B : *AC.getAnalysis<PostOrderCFGView>()) {
worklist.enqueueBlock(B);
----------------
xazax.hun wrote:
> With `BackwardDataflowWorklist`, each `enqueueBlock` will insert the block
> into a `llvm::PriorityQueue`. So regardless of the insertion order, `dequeue`
> will return the nodes in the reverse post order.
>
> Inserting elements in the right order into the heap might be beneficial is we
> need to to less work to "heapify". But on the other hand, we did more work to
> insert them in the right order, in the first place. All in all, I am not sure
> whether the comment is still valid and whether this patch would provide any
> benefit over the original code.
>
Yes, what Gabor says makes sense. On the other hand I don't see any overhead -
I might be wrong though - in the post order visitation. And it makes the code
more consistent IMHO.
Well, it would be important to know why the original author put the
```
// FIXME: we should enqueue using post order.
```
there. The blamed commit 77ff930fff15c3fc76101b38199dad355be0866b is not saying
much.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87519/new/
https://reviews.llvm.org/D87519
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
