[AIEX]Refactor extract 128-bit subvector legalizer. #690
+30
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
niwinanto
requested review from
F-Stuckmann,
SagarMaheshwari99,
abhinay-anubola,
abnikant,
andcarminati,
katerynamuts,
khallouh,
konstantinschwarz,
martien-de-jong and
stephenneuendorffer
as code owners
| // the second 128-bits to lsb positions of output. | ||
| int64_t ModeLoValue; | ||
| int64_t ModeHiValue; | ||
| if (ST.isAIE2P()) { |
There was a problem hiding this comment.
I think we can const-initialize the modes here:
auto GetShuffleModes = [&] () {
if (ST.isAIE2P())
return std::make_pair(/*Lo*/ 8, /*Hi*/ 9);
else
llvm_unreachable("vshuffle mode value needed for target.");
};
const auto [ModeLoValue, ModeHiValue] = GetShuffleModes();
There was a problem hiding this comment.
This is smart. Done!
| MIRBuilder.buildConstant(ModeReg, LaneIdx ? 9 : 8); | ||
| MIRBuilder.buildConstant(ModeReg, ModeHiValue); | ||
| // step 4: Create vshuffle. For LaneIdx 0, we dont have to use the | ||
| // shuffle, padded register itself is enough. |
There was a problem hiding this comment.
I mean, we pad 128-bit register to 512 in step 1. We could directly use it.
| ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<2 x s128>) = COPY $wl0 | ||
| ; CHECK-NEXT: [[BITCAST:%[0-9]+]]:_(<32 x s8>) = G_BITCAST [[COPY]](<2 x s128>) | ||
| ; CHECK-NEXT: [[AIE_PAD_VECTOR_UNDEF:%[0-9]+]]:_(<64 x s8>) = G_AIE_PAD_VECTOR_UNDEF [[BITCAST]](<32 x s8>) | ||
| ; CHECK-NEXT: [[DEF:%[0-9]+]]:_(<64 x s8>) = G_IMPLICIT_DEF |
|
It looks great, I left few comments. |
niwinanto
force-pushed
the
niwin.vshuffle.legalizer
branch
from
7f0f8b1 to
71f80b8
Compare
| // step 4: Create vshuffle. For LaneIdx 0, we dont have to use the | ||
| // shuffle, padded register itself is enough. | ||
| if (LaneIdx) { | ||
| ShuffleInstr = |
There was a problem hiding this comment.
nit: ShuffleOrCopyInstr.
| const unsigned LaneIdx = IdxVal->Value.getZExtValue(); | ||
| MIRBuilder.buildConstant(ModeReg, LaneIdx ? 9 : 8); | ||
| MIRBuilder.buildConstant(ModeReg, ModeHiValue); | ||
| // step 4: Create vshuffle. For LaneIdx 0, we dont have to use the |
| const Register ModeHiReg = | ||
| MIRBuilder.buildConstant(S32, ModeHiValue).getReg(0); | ||
| MIRBuilder.buildSelect(ModeReg, IdxReg, ModeHiReg, ModeLoReg); | ||
| // step 4: Create vshuffle |
There was a problem hiding this comment.
nit: ... vshuffle.
(dot)
Since shuffle modes are different for different AIE targets, refactored the use in legalizer. Also, extract subvector 128-bit doesn`t need a vshuffle, if we want to extract from idx 0 and known at compile time.
niwinanto
force-pushed
the
niwin.vshuffle.legalizer
branch
from
71f80b8 to
524493c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since shuffle modes are different for different AIE targets, refactored the use in legalizer. Also, extract subvector 128-bit doesn`t need a vshuffle, if we want to extract from idx 0 and known at compile time.