[PATCH] D16306: [TTI] Add getCacheLineSize

Adam Nemet via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 20 13:55:29 PST 2016


anemet added inline comments.

================
Comment at: include/llvm/Analysis/TargetTransformInfoImpl.h:267
@@ -266,1 +266,3 @@
 
+  unsigned getCacheLineSize() { return 64; }
+
----------------
hulx2000 wrote:
> anemet wrote:
> > hulx2000 wrote:
> > > Return 64 by default is not good, it would be better if assert or return unsupported message by default, and only return 64 for PPC.
> > > 
> > > 
> > I don't see a precedent for doing such invasive things in the TTI layer itself.
> > 
> > I could return 0 (invalid).  That way clients can do the assert or disable any action based on this parameter.
> > 
> > 
> My concern is : 
> 
> Now only PPC use this API, which is fine no matter what you do.
> 
> However if you return 64 by default, some day other cpu/architecture will start to use it, if people are not careful enough, it will return possibly wrong value for them, give some warning/assert is better than silently return a wrong value, return 0 is ok too.
OK, 0 sounds like a good compromise.  Adding it.


http://reviews.llvm.org/D16306





More information about the llvm-commits mailing list