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

Wed Sep 16 11:02:07 PDT 2020

fhahn added a comment.

In D87163#2276899 <https://reviews.llvm.org/D87163#2276899>, @dmajor wrote:

> In D87163#2275896 <https://reviews.llvm.org/D87163#2275896>, @asbirlea wrote:
>
>> I checked in a fix in https://reviews.llvm.org/rGfc8200633122, but I haven't yet verified it addresses all the failures reported.
>
> Thanks, I've confirmed that this fixes our tests in their original form (not reduced).
>
> Our day-to-day testing of LLVM trunk is limited though, maybe one-tenth of our full suite. Since this code has some risks, I could start a full run to throw more testing at it. But that takes more machine capacity and human effort to remove flaky failures, so I'd prefer to wait until the odds are good that there won't be further changes. Let me know when you think it's ready.

I think I managed to come up with another test case that unfortunately is not covered by the recent fix. I think we might need to be even more conservative when walking across MemoryPhis and put up D87778 <https://reviews.llvm.org/D87778>, which includes the test case, which is crafted so that the result of PHI translation dominate the relevant blocks during translation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87163