[PATCH] D16306: [TTI] Add getCacheLineSize

Lawrence Hu via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 20 11:35:47 PST 2016


hulx2000 added inline comments.

================
Comment at: include/llvm/Analysis/TargetTransformInfoImpl.h:267
@@ -266,1 +266,3 @@
 
+  unsigned getCacheLineSize() { return 64; }
+
----------------
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.


http://reviews.llvm.org/D16306





More information about the llvm-commits mailing list