[PATCH] D30350: [LSR] Add a cap for reassociation of AllFixupsOutsideLoop type LSRUse to protect compile time

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 16 22:26:22 PDT 2017


sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm with nits



================
Comment at: include/llvm/Analysis/ScalarEvolution.h:1163
+
+  typedef SmallDenseMap<std::pair<const SCEV *, Type *>, const SCEV *, 8>
+      CacheTypeForExtend;
----------------
I think the usual naming scheme here is `ExtendCacheTy`?


================
Comment at: lib/Analysis/ScalarEvolution.cpp:1522
+  const SCEV *ZExt = getZeroExtendExprImpl(Op, Ty, Cache);
+  Cache.insert({{Op, Ty}, ZExt});
+  return ZExt;
----------------
If possible, you should

```
auto InsertResult = Cache.insert({{Op, Ty}, ZExt});
assert(InsertResult.second);
```

otherwise state that it is possible (how?) that the key is already present in the cache.


Repository:
  rL LLVM

https://reviews.llvm.org/D30350





More information about the llvm-commits mailing list