[PATCH] D88982: [NFCI] Add PolyBase as base class for ElementCount, TypeSize and StackOffset.

Christopher Tetreault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 7 14:06:30 PDT 2020


ctetreau added a comment.

In D88982#2317488 <https://reviews.llvm.org/D88982#2317488>, @sdesmalen wrote:

> Are you suggesting rename PolyBase or just to remove the term 'polynomial' from the commit message and any of the comments?

I'm suggesting a rename. According to wikipedia, a polynomial of first degree is called a linear polynomial. So, I propose the following simplified hierarchy:

  LinearPolyBase<Type, Dimensions, IsUnivariate>
     ^   
     |   
     +---- StackOffset : LinearPolyBase<int64_t, 2, false>
     ^
     |   
     +---- PolySize : LinearPolyBase<Type, 2, true>
                  ^   
                  |   
                  +------------- ElementCount (typedef with Type=unsigned)
                  |   
                  +------------- TypeSize (subclass with Type=uint64_t)

If we do this, and don't define operator* with another linear polynomial on the right, then we should be good to go. I don't think MixedPolyBase2D bought us anything, all the new things you added were "scalable" specific anyway. Similarly, for SinglePolyBase2D: might as well just let it be a "scalable size" IMO.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88982/new/

https://reviews.llvm.org/D88982



More information about the llvm-commits mailing list