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

[Daniel Berlin via llvm-commits]

On Mon, Jul 31, 2017 at 8:08 PM, Serguei Katkov via Phabricator <
reviews at reviews.llvm.org> wrote:

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

It, looked, at a glance, like you may visit the same phi nodes again and
again for different candidates.
But maybe i misread.


>
> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170731/30616b1a/attachment.html>