[llvm-commits] [Review] GVN Patch

[Bill Wendling]

On Oct 19, 2012, at 3:37 PM, Eli Friedman <eli.friedman at gmail.com> wrote:

> On Fri, Oct 19, 2012 at 3:28 PM, Owen Anderson <resistor at mac.com> wrote:
>> 
>> On Oct 19, 2012, at 3:22 PM, Bill Wendling <wendling at apple.com> wrote:
>> 
>> Owen expressed doubt about that when I asked him because memdep uses caching
>> and stuff. Owen, could you shed some light on what your concerns were?
>> 
>> 
>> My concern here is that MemDep performs caching, which would then be
>> invalidated if the set of unreachable blocks changes.  Perhaps this  is not
>> an issue, if we're already having to invalidate the cache in those
>> circumstances already; it's been a long time since I looked at it.
> 
> We don't invalidate that cache currently.  That said, the relevant
> callback already exists, so we can just blow away a bit more state if
> necessary.
> 
> Another issue with putting the check in GVN: if MemDep can look at
> unreachable blocks, it can look through unreachable blocks, so we
> could end up returning completely non-sensical results in reachable
> blocks.
> 
It doesn't look like we're removing basic blocks except when they're merged together, in which case the called function invalidates the MD cache. It seems "safe" to place the check inside memdep, which is preferable...

-bw