[PATCH] D40480: MemorySSA backed Dead Store Elimination.

[JF Bastien via Phabricator via llvm-commits]

jfb added a comment.

>> Can you add tests for `volatile`?
> 
> The idea here would be that that enable-dse-memoryssa would be set to true eventually, so this should pass every existing DSE test. The old parts of this file could then be deleted. The multiblock tests are new as that's not something the old pass could do. I'm not sure if this will still pass all the existing tests, or if the old DSE algorithm has learnt new tricks since then.

Old LLVM code is often poorly tests. The DSE directory has two volatile stores, one volatile load, and no volatile atomics. It has a decent amount of atomic load / store, and no atomic RMW or cmpxchg. I agree that you want to pass old tests (potentially with tweaks as some things change), but the current volatile and atomic situation is not solid grounds for an algorithm swap.

I *want* DSE of dead stores around volatile and atomic. I'm not asking for this patch to implement it. I just want basic correctness so that, now and once we start being more aggressive, we know there's no breakage.


https://reviews.llvm.org/D40480