[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