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

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:607
+  /// The possible cache levels
+  enum CacheLevel {
+    CL_L1,   // The L1 cache
----------------
It's conceivable that targets might want to differentiate between the I-cache and D-cache. e.g an embedded target may use info on the I-cache to guide custom code layout optimisations.

Is it worth starting with CL_L1D and CL_L2D instead?


================
Comment at: include/llvm/Analysis/TargetTransformInfoImpl.h:348
+    case TargetTransformInfo::CL_L2:
+      return 256 * 1024; // 128 KByte
+    }
----------------
Either the comment or the calculation is incorrect!


================
Comment at: lib/Target/X86/X86TargetTransformInfo.cpp:92
+    //   - Kabylake
+    return 256 * 1024; // 128 KByte
+  }
----------------
Either the comment or the calculation is incorrect!


================
Comment at: lib/Target/X86/X86TargetTransformInfo.h:49
   /// @}
+  unsigned getCacheSize(TargetTransformInfo::CacheLevel Level) const;
 
----------------
Why not implement getCacheAssociativity too?


https://reviews.llvm.org/D37051