SCCP does not change the CFG, so we can mark it as preserved.
If I recall correctly the sparse constant propagation implementation in LLVM (interproceturally, at least), calls changeToUnreachable which modifies the CFG.
(this is, quite unfortunate, as it's declared in Local.h). So, I'm afraid the patch as is is not correct. That said, it's been a while since I last worked on this area so I might recall incorrectly :)
This is, AFAIK, right but your assumption is wrong.
In particular, DT is preserved, but the CFG is definitely changed by SCCP.
See changeToUnreachable and what it does under the covers.
(this has confused people before :P).
Right, IPSCCP does use changeToUnreachable, but it can be told to update the DT. The thing that confused me is that we use removeAllNonTerminatorInstruction... in runSCCP with a comment that we should not change the CFG. Is there a reason we use changeToUnreachable in IPSCCP but removeAllNonTerminatorInstruction... in SCCP?
Florian, I won't be opposed if you decide to preserve the dominator tree as part of changeToUnreachable, but, as we discussed with Dan in the past, do we really need to? I mean, we could just change changeToUnreachable (no pun intended) to do something sane and not modify the CFG (if the block is really unreachable, it's very likely that the next round of DCE is going to remove it anyway) [and in case it doesn't I'm really not worried about running DCE again because it's really basically free].