[PATCH] D37051: Model cache size and associativity in TargetTransformInfo

[Tobias Grosser via Phabricator via llvm-commits]

grosser added a comment.

In https://reviews.llvm.org/D37051#849798, @gareevroman wrote:

> > I think throughput and latency of vector fma instructions are pretty constant across micro-architectures too. Can we also add them?
>
> Sorry, probably, it’d require to specify it for each architecture.


We could do this. Some of the backend experts might be able to help you how to do this best. I think we should do this in a separate patch. Care to propose one?



================
Comment at: include/llvm/Analysis/TargetTransformInfoImpl.h:343
 
+  unsigned getCacheSize(TargetTransformInfo::CacheLevel Level) {
+    switch (Level) {
----------------
asb wrote:
> fhahn wrote:
> > Maybe it would be safer to be conservative and return 0 here, similar to what getCacheLineSize does currently? That allows passes to check if the target provides accurate information ( != 0). 
> > 
> > For example, for ARM Cortex cores the level 1 cache size can vary between 0KByte and 64KByte. [1], [2]
> > 
> > Also, we already have getCacheLineSize. Would it make sense to express the cache size in terms of cache lines, (eg X * Cache line size)? That would make it slightly easier to keep them in sync and avoid situations where getCacheSize() is not a multiple of getCacheLineSize()
> > 
> > 
> > [1] https://en.wikipedia.org/wiki/Comparison_of_ARMv8-A_cores
> > [2] https://en.wikipedia.org/wiki/ARM_Cortex-M
> I wondered about returning 0 too, but also think we need a new sentinel value. This function should be able to indicate 1) cache is known size X, 2) cache is unknown size, 3) cache is known size 0 (e.g. there is no L2).
I changed this to llvm::Optional.


https://reviews.llvm.org/D37051