[AIEX][AIE2P] Vshift (Vshift) to CONCAT combiner #772
Conversation
andcarminati
requested review from
F-Stuckmann,
SagarMaheshwari99,
abhinay-anubola,
abnikant,
katerynamuts,
khallouh,
konstantinschwarz,
martien-de-jong,
mludevid,
niwinanto and
stephenneuendorffer
as code owners
andcarminati
force-pushed
the
andreu.shift.combiner
branch
4 times, most recently
from
3b74690 to
2fbd949
Compare
|
QoR: |
| if (Opcode == TII.getGenericVSelOpcode()) { | ||
| const Register MaskReg = User.getOperand(3).getReg(); | ||
| const auto MaskVal = getIConstantVRegVal(MaskReg, MRI); | ||
| if (MaskVal) { |
There was a problem hiding this comment.
we can reduce nesting if we directly reutn std::nullopt if !MaskVal
There was a problem hiding this comment.
Yeah, definitely.
| const unsigned Opcode = User.getOpcode(); | ||
|
|
||
| // VSEL: Check which elements are selected via the mask | ||
| if (Opcode == TII.getGenericVSelOpcode()) { |
There was a problem hiding this comment.
We may want to move these big if conditions into lambdas, so that the high level flow of the function is clearer
| const uint64_t Mask = MaskVal->getZExtValue(); | ||
| if (Mask != 0) { | ||
| const unsigned HighestBit = 63 - llvm::countl_zero(Mask); | ||
| MaxElement = std::max(MaxElement, HighestBit); |
There was a problem hiding this comment.
do we expect multiple vsels in the register users?
There was a problem hiding this comment.
We look to all the users, it is too restrictive to assume just one user.
andcarminati
force-pushed
the
andreu.shift.combiner
branch
from
2fbd949 to
0780d74
Compare
| const Register IdxReg = User.getOperand(2).getReg(); | ||
| const auto Idx = getIConstantVRegVal(IdxReg, MRI); | ||
| if (Idx) { | ||
| MaxElement = |
There was a problem hiding this comment.
why don't we return here? why should we have multiple G_EXTRACT_VECTOR_ELT s coupled with G_UNMERGE_VALUES and return the maximum? aren't we then confusing combine patterns?
There was a problem hiding this comment.
You can see the name of the function, this is just a helper function that we use to try prove that the upper part of a vector is not used, so we cannot just return max here because we may have other users accessing even higher elements. But, maybe you are talking about an early return in case if !Idx - this I agree, it will be part of the next version.
2 participants
The combiner eliminates a VSHIFT chain and replaces it with a simpler G_CONCAT_VECTORS operation with zero-padded upper half. The transformation is generalized to work with any vector size by calculating expected shift amounts based on source and destination types (e.g., for 512 bit expansion: shift1=16 bytes, shift2=48 bytes). The combiner includes usage-aware optimization: it analyzes how the result vector is actually used (via a new getMaxUsedVectorElement utility) and chooses between padding with UNDEF or zeros.
This PR also includes two redundant pad/unpad removal combiners.