[PATCH] D52243: [ConstHoist] Do NOT rebase single (or few) dependent of base insertion point

[Z. Zheng via Phabricator via llvm-commits]

zzheng marked 3 inline comments as done.
zzheng added inline comments.


================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:873
+      if (ToBeRebased.size() < MinNumOfDependentToRebase) {
+        Base->eraseFromParent();
+        NotRebasedNum += ToBeRebased.size();
----------------
dmgreen wrote:
> Can you change this around so we don't speculatively make the Base? That seems to be the way of doing things in LLVM.
Here Base is not the base constant, but an instance (insertion point) of it. A base constant can have multiple insertion points. The existing algorithm first finds the insertion points, then looks for rebased uses dominated by each insertion point.

If I understand your comments correctly, it's desirable to not to have Base at the first place so no need to erase it later. But we need the number of rebased uses to determine if we'll rewrite such uses. To get this number we need Base.

I can remove the statement:
```
Base->eraseFromParent();
```
Since Base has no use, it'll be trivially removed by DCE.


Repository:
  rL LLVM

https://reviews.llvm.org/D52243