[PATCH] D87163: [DSE] Switch to MemorySSA-backed DSE by default.

Mon Sep 14 05:52:51 PDT 2020

uabelho added a comment.

In D87163#2270918 <https://reviews.llvm.org/D87163#2270918>, @uabelho wrote:

> In D87163#2270860 <https://reviews.llvm.org/D87163#2270860>, @fhahn wrote:
>
>> In D87163#2270568 <https://reviews.llvm.org/D87163#2270568>, @uabelho wrote:
>>
>>> Hi,
>>>
>>> I'm seeing what I think is a miscompile with this:
>>
>> Thanks for the reproducer. We have to be more careful around pointers that may reference multiple locations in memory. I pushed f715d81c9df3 <https://reviews.llvm.org/rGf715d81c9df3fb3e047a54899fc749f57c84aeb5> which limits elimination to stores that only reference a single location (or are in the same BB). The check is conservative and can be improved in the future, but the impact on the number of stores eliminated seems very low in practice. It would be great if you could check if that fixes the original reproducer.
>
> Great! I'll check and get back during the day.

Yep, the original reproducer passes now. Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163