[PATCH] D30322: [SCCP] Resolve indirect branch target when possible.

[Davide Italiano via Phabricator via llvm-commits]

davide added inline comments.


================
Comment at: lib/Transforms/Scalar/SCCP.cpp:1531-1548
+   if (auto *IBR = dyn_cast<IndirectBrInst>(TI)) {
+      if (!getValueState(IBR->getAddress()).isUnknown())
+        continue;
+
+      // If the input to SCCP is actually branch on undef, fix the undef to
+      // the first successor of the indirect branch.
+      if (isa<UndefValue>(IBR->getAddress())) {
----------------
trentxintong wrote:
> davide wrote:
> > I'm very wary of adding more code here, as it caused a fair amount of pain in the past. This, tho, seems to be in line with what was already there.
> I feel getting rid of this undef resolver is the way to go for SCCP. When I read this pass, I do not feel very good about how undef is resolved, especially the ordering terminators and its input is resolved does not seem to be bullet-proof.  And it does have bugs.
> 
> However this is what we need to get indirectbr simplification working for now =).
I really agree, and in fact I discussed this on the mailing lists (and with Dan Berlin and David Majnemer). I have a patch for that, but needs some performance testing. If you care about this, maybe you can run on your benchmarks? (I'm going to clean up the branch and send the patch to you).


https://reviews.llvm.org/D30322