[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