[PATCH] D6594: Fix for bug 8281 - Extremely slow assembling and disassembling of ptrtoint

[Tim Shen via Phabricator via llvm-commits]

timshen added inline comments.


================
Comment at: llvm/lib/IR/ConstantFold.cpp:365
+    Constant *E = getFoldedSizeOf(ATy->getElementType(), DestTy, true, Cache);
+    return Cache[Ty] = ConstantExpr::getNUWMul(E, N);
   }
----------------
timshen wrote:
> This is correct today, but newly added `return` statements in the future might miss the `Cache[Ty] = ` part.
> 
> Do you mind doing the following:
> ```
> static Constant *getFoldedSizeOfImpl(Type *Ty, Type *DestTy, bool Folded,
>                                  DenseMap<Type *, Constant *> &Cache) {
>   // The actual implementation, no explicit caching.
>   // It recursively calls into getFoldedSizeOf below.
> }
> 
> static Constant *getFoldedSizeOf(Type *Ty, Type *DestTy, bool Folded,
>                                  DenseMap<Type *, Constant *> &Cache) {
>   Constant*& V = Cache[Ty];
>   if (V == nullptr)
>     V = getFoldedSizeOfImpl(...);
>   return V;
> }
> ```
```
  Constant*& V = Cache[Ty];
  if (V == nullptr)
```

Actually we should use `result = Cache.insert(...); if (result.second) ...` instead, since `nullptr` might be a valid cached value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D6594