-
Notifications
You must be signed in to change notification settings - Fork 556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CPU] Remove unnecessary factors from getMaxVectorTileSize. #15843
Conversation
There is no lower_bound and upper_bound context in vector tile size. We should just use the number of elements factor. It removes `lb` and `ub`; it asks users to provide the number of elements directly. It also removes `allowIncomplteTile` because this factor should already be modeld by `enforcePowerOfTwo`.
63d1b46
to
b8bcec8
Compare
I want to claim this as an NFC but I remove the logics about We likely don't hit the branch when computing vector sizes, so it does not impact any lit tests and benchmark results. It looks like a good cleanup to me. @dcaballe what do you think? |
@@ -854,9 +838,6 @@ static LogicalResult setMatmulNoPadRootConfig( | |||
const SmallVectorImpl<bool> &vecScalableDims = inputScalableTileFlags.back(); | |||
SmallVector<int64_t> parallelTileSizes; | |||
SmallVector<bool> parallelScalableFlags; | |||
bool allowIncompleteTile = | |||
vecPreProcStrategy == VectorPreProcStrategy::Peeling || | |||
vecPreProcStrategy == VectorPreProcStrategy::Masking; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to assert if the strategy is not one of these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is okay because all the matmul strategy is using peeling or masking. At least it is the case for targets we've been actively working on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SG, thanks!
There is no lower_bound and upper_bound context in vector tile size. We should just use the number of elements factor. It removes
lb
andub
; it asks users to provide the number of elements directly.It also removes
allowIncomplteTile
because this factor should already be modeld byenforcePowerOfTwo
.