<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 31, 2017 at 8:08 PM, Serguei Katkov via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">skatkov added inline comments.<br>
<span class=""><br>
<br>
================<br>
Comment at: lib/CodeGen/CodeGenPrepare.<wbr>cpp:4381<br>
+// Try to match PHI node to Candidate. Matcher tracks the matched Phi nodes.<br>
+static bool MatchPhiNode(PHINode *PHI, PHINode *Candidate,<br>
+                         DenseSet<PHIPair> &Matcher,<br>
----------------<br>
</span><span class="">dberlin wrote:<br>
> FWIW, i'd just hash them, rather thancheck the possible matches again and again, i think.<br>
><br>
> Happy to give you some code to crib for that.<br>
><br>
><br>
</span>Interesting idea, you mean hash the Phi node and compare hashes first? I'm not sure I'm doing comparison of the same Phi node many times.<br></blockquote><div><br></div><div>It, looked, at a glance, like you may visit the same phi nodes again and again for different candidates.</div><div>But maybe i misread.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I take a Phi node and compare it against each Phi in the same basic block. If I find a match I will recursively check the dependency.<br>
If this Phi node is not matched to any phi node in this basic block, I just bail out if creation of new Phi node is not allowed or move this Phi node (and all other Phi nodes considered together) to the list of known Phi nodes. And I will not consider them again.<br>
<br>
So it seems that hashing is redundant here.<br>
Do I miss anything?<br>
<br>
<br>
<a href="https://reviews.llvm.org/D36073" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D36073</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>