baloghadamsoftware 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);
----------------
martong wrote:
> 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.
Please compare the execution time with and without this patch. I think it is
difficult do decide in theory which one costs more: the heapification during
insertion or the reverse ordering before the insertion.
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
