[PATCH] D121744: [SCCP] Update ValueLatticeElement blockaddresses when removing unreachable BasicBlocks

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers planned changes to this revision.
nickdesaulniers added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:534-543
+          // If the use is a ConstantExpr, construct a new ConstantExpr which
+          // uses `inttoptr i32 1 to i8*` instead of the `blockaddress`.
+          if (auto *CE = dyn_cast<ConstantExpr>(U)) {
+            std::vector<Constant *> Ops;
+            for (Use &Op : CE->operands())
+              Ops.push_back(Op == BA ? C : cast<Constant>(Op));
+            Constant *C = CE->getWithOperands(Ops);
----------------
While this removes the special case for bitcasts, it's still subtly not quite right; it only checks for one level of nesting of `ConstExpr`s; there could be multiple "levels of nesting" for the aggregated `Constant` stored in the lattice.

i.e. the `BlockAddress` of interest might have the following `User`: `i64* bitcast (i8* blockaddress(@main, %redirected) to i64*)`; but I worry if the lattice had something like `i8* bitcast (i64* bitcast (i8* blockaddress(@main, %redirected) to i64*) to i8*)`; then we'd be trying to replace the former and not finding the latter. Unless the latter is also considered a `User` of the `BlockAddress`?

I can test that, but I'm going to see first whether we can query the solver whether it has reach-ability info ready BEFORE we internalize new `ConstantExpr`'s containing `BlockAddress` `Constant`s into the lattice.  Then we could avoid this find+replace dance and simply do less work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121744/new/

https://reviews.llvm.org/D121744