[PATCH] D32140: Global code motion of congruent computations

Tue Oct 29 07:11:38 PDT 2019

hiraditya added a comment.

In D32140#1725210 <https://reviews.llvm.org/D32140#1725210>, @fhahn wrote:

> Two high level comments after a quick glance
>
> 1. gnv-sink & gvn-hoist are still disabled by default AFAIK and there are still a few known bugs that need to be addressed (https://bugs.llvm.org/buglist.cgi?quicksearch=gvn-hoist, https://bugs.llvm.org/buglist.cgi?quicksearch=gvn-sink). It would be good to guard this change by a new flag that's off by default, otherwise it might hinder work towards weeding out the existing bugs.

Thanks for the link, I'll look at the bugs and try to fix relevant ones

> 2. Do you think you would be able to use the MergeSet implementation from D57123 <https://reviews.llvm.org/D57123>  (I will rebase the patches ASAP)? Separating out the MergeSet changes should reduce the diff a bit, making reviewing it easier.

Sure, that would help simplify the code here. I haven't looked at the implementation (D57123 <https://reviews.llvm.org/D57123>) in detail but I'll follow up after it is rebased.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D32140/new/

https://reviews.llvm.org/D32140