[PATCH] D36073: [CGP] Extends the scope of optimizeMemoryInst optimization

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4381
+// Try to match PHI node to Candidate. Matcher tracks the matched Phi nodes.
+static bool MatchPhiNode(PHINode *PHI, PHINode *Candidate,
+                         DenseSet<PHIPair> &Matcher,
----------------
dberlin wrote:
> FWIW, i'd just hash them, rather thancheck the possible matches again and again, i think.
> 
> Happy to give you some code to crib for that.
> 
> 
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.

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.
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.

So it seems that hashing is redundant here.
Do I miss anything?


https://reviews.llvm.org/D36073