[PATCH] D49408: [SCCP] [WIP] Don't use markForcedConstant on branch conditions.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

I just collected stats with this patch on the test-suite with LTO. The only difference in stats counters across IPSCCP and SCCP is that with this patch, we get 2 more instructions eliminated in SCCP (sccp.NumInstRemoved).

Thinking about it, I am not too surprised that the impact of not forcing the condition to false is tiny. IIUC the only benefit would be if our guess (condition false) is actually true and the condition is used somewhere in the else branch. As mentioned in the discussion at https://reviews.llvm.org/D49384, this helps us to avoid obscure bugs caused by back-propagating information to the condition and resolves  https://reviews.llvm.org/D49384 and https://reviews.llvm.org/D48327. I think we could come back to actually distinguishing between dead code and an LLVM "undef" value once there is a bigger need, e.g. because IPSCCP became more powerful



================
Comment at: lib/Transforms/Scalar/SCCP.cpp:1594
+      BasicBlock *DefaultSuccessor = TI->getSuccessor(1);
+      if (!KnownFeasibleEdges.count({&BB, DefaultSuccessor})) {
+        markEdgeExecutable(&BB, DefaultSuccessor);
----------------
markEdgeExecutable also checks if the edge is already known to be feasible. Maybe it's worth changing markEdgeExecutable to return false, when a unfeasible edge is passed?


Repository:
  rL LLVM

https://reviews.llvm.org/D49408