[PATCH] D75634: [ConstantFolding] Accept FoldedOps cache as parameter

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

The point of the cache in ConstantFolding is really to avoid bad behavior for constant expression trees with a "diamond" shape; in that context, it doesn't really matter what level it starts at, since constants have a limited number of operands.  But sure, if we have the cache anyway, we might as well use it.

I'd like to see some sort of performance measurement here, given we're complicating the ConstantFoldConstant API for the sake of performance.



================
Comment at: lib/Analysis/ConstantFolding.cpp:1192
+                           SmallDenseMap<Constant *, Constant *> &FoldedOps) {
+  if (!isa<ConstantVector>(C) && !isa<ConstantExpr>(C))
+    return const_cast<Constant *>(C);
----------------
Do you have any idea why we're checking for ConstantVector specifically, instead of ConstantAggregate?


================
Comment at: lib/Analysis/ConstantFolding.cpp:1195
+
+  auto It = FoldedOps.find(C);
+  if (It != FoldedOps.end())
----------------
Might as well use DenseMap::lookup() here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75634