[llvm-commits] [PATCH] Reduce complexity of DAGCombiner

[Chris Lattner]

On Feb 15, 2012, at 4:06 AM, James Molloy wrote:

> Hi Chris,
>  
> Thanks for the quick review! J
>  
> 1.       I’ve used SmallPtrSet with a size of 64 as a wild finger-in-the-air guess. Do you reckon that’s about right?
>  
> 2.       Fixed, thanks.
>  
> 3.       Omission on my part – removed during my changes and forgot to re-add it when I found it was actually rather important L
>  
> 4.       I’ve modified the comments. They still compare the algorithm versus a naive/obvious approach because I feel that I have to justify the extra complexity. Are they OK now?

Looks great, please commit.  As a follow-on micro-optimization, you can eliminate a redundant set lookup in this loop:

+    do {
+      N = WorkListOrder.back();
+      WorkListOrder.pop_back();
+    } while(WorkListContents.count(N) == false);
+    WorkListContents.erase(N);

By changing it to:

 // Iterate until we successfully remove an element in the set.
 do {
   N = WorkListOrder.back();
   WorkListOrder.pop_back();
 } while (!WorkListContents.erase(N));

-Chris


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120215/d8fb7059/attachment.html>