[PATCH] D44282: [PR16756] JumpThreading: explicitly update SSA rather than use SSAUpdater.

Davide Italiano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 8 17:30:46 PST 2018


davide added a comment.

I'm going to review this in detail, but I have a general comment about the design.
I'm a little torn about the approach you're using. I think it's a good experiment because you're leveraging the structure of the problem to find a fundamentally more efficient solution. This is, FWIW, not really surprising.
OTOH, this is bad because you're de-facto rewriting a specialized SSA updater. We might need another one for, e.g. LCSSA, and another one for GVN/PRE (until NewGVN is in or we rewrite PRE to not use the updater).
This results in basically a bunch of code duplicated. History shows (and you know this very well) that it turned out to be a long tail of bugs when we did something similar with the dominator (have specialized updates per-pass). If we really want to go this route, we really need to analyze carefully all the implications.


Repository:
  rL LLVM

https://reviews.llvm.org/D44282





More information about the llvm-commits mailing list