[PATCH] D19821: [EarlyCSE] Port to use MemorySSA (disabled by default). NFC.

Geoff Berry via llvm-commits llvm-commits at lists.llvm.org
Tue May 17 08:23:56 PDT 2016


gberry added a comment.

@reames I've attempted to resolved most of your individual concerns (or at least made them explicit in the change).  The bigger question of whether this is worth the compile time remains to be determined.  Do you think more tests need to be added in addition to the already existing EarlyCSE tests?  Adding additional run lines to those tests to enable -early-cse-use-memoryssa seems like overkill to me, but I don't feel to strongly about it.  Or are you more concerned about adding new tests for cases that are only caught by MemorySSA (both positive and negative)?

@dberlin, @george.burgess.iv There are a couple of FIXME comments in this change that identify cases where MemorySSA is maybe being too conservative (e.g. when dealing with fence release instructions and load atomic instructions).  Do you think it is reasonable to refine these cases in MemorySSA or is the conservatism restricted to EarlyCSE's usage, in which case we should deal with it in EarlyCSE?  Similarly, what do you think of Phillip's suggestion to look at using ValueHandles in MemorySSA to make removal invalidating more automated?


http://reviews.llvm.org/D19821





More information about the llvm-commits mailing list