Skip to content

Conversation

@victor-eds
Copy link
Contributor

Detect sub-group shuffle convert_layout cases of more than one element per thread.

@victor-eds victor-eds requested review from a team, etiotto and whitneywhtsang October 25, 2024 10:53
@victor-eds victor-eds self-assigned this Oct 25, 2024
Comment on lines +531 to +541
int32_t registerInDimSize = conversion->getInDimSize(kRegister);
int32_t laneInDimSize = conversion->getInDimSize(kLane);
return conversion->getBases().lookup(kRegister) ==
buildSubGroupTransposeRegisterBases(registerInDimSize,
laneInDimSize) &&
conversion->getBases().lookup(kLane) ==
buildSubGroupTransposeLaneBases(laneInDimSize);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor and protected against some illegal layout creation.


LinearLayout comp = dstLayout.invertAndCompose(srcLayout);
std::optional<LinearLayout> conversion = comp.divideRight(
LinearLayout::zeros1D(comp.getInDimSize(kLane), kLane, kLane) *
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should always be 0 for this case

Comment on lines 647 to 648
if (!conversion)
return false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be None is 0 check fails.

@victor-eds victor-eds marked this pull request as draft October 25, 2024 12:40
@victor-eds victor-eds marked this pull request as ready for review October 25, 2024 12:48
Detect sub-group shuffle `convert_layout` cases of more than one element per thread.

Signed-off-by: victor-eds <[email protected]>
@victor-eds victor-eds force-pushed the sub-group-shuffle-broadcast-extend branch from cb1d9f5 to a7277b1 Compare October 28, 2024 16:00
Comment on lines +674 to +676
conversion->getBases().lookup(kRegister) ==
buildSubGroupShuffleRegisterBases(registerInDimSize,
laneOutDimSize);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will investigate whether there is a better way to express this in the future when generalizing. Same for transpose case.

Copy link

@mfrancepillois mfrancepillois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing some background on subgroup shuffle, but PR LGTM.

@victor-eds victor-eds enabled auto-merge (squash) October 30, 2024 16:47
@victor-eds victor-eds merged commit 5d9774c into intel:main Oct 30, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants