It looks like the DeadStoreElimination optimization doesn't work across BasicBlock boundaries. The project I'm working on (https://github.com/trailofbits/mcsema), would tremendously benefit from even simple cross-block DSE.
If you have specific small examples which aren’t getting caught, I’d be interested in seeing test cases (as bugs.) EarlyCSE handles limited cases here. GVN probably could, but doesn’t seem to. (Which surprises me.) I suspect it got lost in review. It looks like there were some quality issues with the original implementation and the cycle never got closed.
I’ve attached two simplified examples: does_optimize.ll and does_not_optimize.ll.
In both examples, the load/store of %STACK_BASEVAL and %STACK_LIMITVAL should be removed, since the values are never used. The difference is that in does_optimize.ll, the store is in the same block as the load, and in does_not_optimize.dll, the store is in a different block.
To do it “right”, you’d have to build SSU and do PRE in the reverse direction, which GVN doesn’t do. The patch posted actually does this “right” (building SSU and using it), though i haven’t looked through it to see if it just does SSU building and usage, or actually does PRE. Either way, it’s a good enough start
For normal cross-block cases, GVN does not value number loads or stores, so it will never discover this, unless it can do so through the load’s direct dependency.
IMHO, cleaning up the SSU based patch and getting it in would be the right way to go. It may require some post-dominance speedups and cleanups to get it fast enough and right enough (I know everyone hates the post-dominator tree, but there is no other way to do proper store sinking)
