[RISCV] Use slideup to lower build_vector when all operand are (extract_element X, 0) #154450
+444
−17
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4513,41 +4513,104 @@ static SDValue lowerBUILD_VECTOR(SDValue Op, SelectionDAG &DAG, | |||||
|
|
||||||
| const unsigned Policy = RISCVVType::TAIL_AGNOSTIC | RISCVVType::MASK_AGNOSTIC; | ||||||
|
|
||||||
| // General case: splat the first operand and slide other operands down one | ||||||
| // by one to form a vector. Alternatively, if every operand is an | ||||||
| // extraction from element 0 of a vector, we use that vector from the last | ||||||
| // extraction as the start value and slide up instead of slide down. Such that | ||||||
| // (1) we can avoid the initial splat (2) we can turn those vslide1up into | ||||||
| // vslideup of 1 later and eliminate the vector to scalar movement, which is | ||||||
| // something we cannot do with vslide1down/vslidedown. | ||||||
| // Of course, using vslide1up/vslideup might increase the register pressure, | ||||||
| // and that's why we conservatively limit to cases where every operand is an | ||||||
| // extraction from the first element. | ||||||
| SmallVector<SDValue> Operands(Op->op_begin(), Op->op_end()); | ||||||
| SDValue EVec; | ||||||
| bool SlideUp = false; | ||||||
| auto getVSlide = [&](EVT ContainerVT, SDValue Passthru, SDValue Vec, | ||||||
| SDValue Offset, SDValue Mask, SDValue VL) -> SDValue { | ||||||
| if (SlideUp) | ||||||
| return getVSlideup(DAG, Subtarget, DL, ContainerVT, Passthru, Vec, Offset, | ||||||
| Mask, VL, Policy); | ||||||
| return getVSlidedown(DAG, Subtarget, DL, ContainerVT, Passthru, Vec, Offset, | ||||||
| Mask, VL, Policy); | ||||||
| }; | ||||||
|
|
||||||
| // The reason we don't use all_of here is because we're also capturing EVec | ||||||
| // from the last non-undef operand. If the std::execution_policy of the | ||||||
| // underlying std::all_of is anything but std::sequenced_policy we might | ||||||
| // capture the wrong EVec. | ||||||
| for (SDValue V : Operands) { | ||||||
| using namespace SDPatternMatch; | ||||||
| SlideUp = V.isUndef() || sd_match(V, m_ExtractElt(m_Value(EVec), m_Zero())); | ||||||
|
There was a problem hiding this comment. Note that even we have interleaving undef "intervals", in the worst case the number of those intervals will only be one more than the number of non-undef values. |
||||||
| if (!SlideUp) | ||||||
| break; | ||||||
| } | ||||||
|
|
||||||
| if (SlideUp) { | ||||||
| MVT EVecContainerVT = EVec.getSimpleValueType(); | ||||||
| // Make sure the original vector has scalable vector type. | ||||||
| if (EVecContainerVT.isFixedLengthVector()) { | ||||||
| EVecContainerVT = | ||||||
| getContainerForFixedLengthVector(DAG, EVecContainerVT, Subtarget); | ||||||
| EVec = convertToScalableVector(EVecContainerVT, EVec, DAG, Subtarget); | ||||||
| } | ||||||
|
|
||||||
| // Adapt EVec's type into ContainerVT. | ||||||
| if (EVecContainerVT.getVectorMinNumElements() < | ||||||
| ContainerVT.getVectorMinNumElements()) | ||||||
| EVec = DAG.getInsertSubvector(DL, DAG.getUNDEF(ContainerVT), EVec, 0); | ||||||
| else | ||||||
| EVec = DAG.getExtractSubvector(DL, ContainerVT, EVec, 0); | ||||||
|
|
||||||
| // Reverse the elements as we're going to slide up from the last element. | ||||||
| std::reverse(Operands.begin(), Operands.end()); | ||||||
|
There was a problem hiding this comment. Nit, does llvm::reverse work here?
Suggested change
There was a problem hiding this comment.
That was actually what I thought, but unfortunately, |
||||||
| } | ||||||
|
|
||||||
| SDValue Vec; | ||||||
| UndefCount = 0; | ||||||
| for (SDValue V : Operands) { | ||||||
| if (V.isUndef()) { | ||||||
| UndefCount++; | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| // Start our sequence with either a TA splat or extract source in the | ||||||
| // hopes that hardware is able to recognize there's no dependency on the | ||||||
| // prior value of our temporary register. | ||||||
| if (!Vec) { | ||||||
| if (SlideUp) { | ||||||
| Vec = EVec; | ||||||
| } else { | ||||||
| Vec = DAG.getSplatVector(VT, DL, V); | ||||||
| Vec = convertToScalableVector(ContainerVT, Vec, DAG, Subtarget); | ||||||
| } | ||||||
|
|
||||||
| UndefCount = 0; | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| if (UndefCount) { | ||||||
| const SDValue Offset = DAG.getConstant(UndefCount, DL, Subtarget.getXLenVT()); | ||||||
| Vec = getVSlide(ContainerVT, DAG.getUNDEF(ContainerVT), Vec, Offset, Mask, | ||||||
| VL); | ||||||
| UndefCount = 0; | ||||||
| } | ||||||
|
|
||||||
| unsigned Opcode; | ||||||
| if (VT.isFloatingPoint()) | ||||||
| Opcode = SlideUp ? RISCVISD::VFSLIDE1UP_VL : RISCVISD::VFSLIDE1DOWN_VL; | ||||||
| else | ||||||
| Opcode = SlideUp ? RISCVISD::VSLIDE1UP_VL : RISCVISD::VSLIDE1DOWN_VL; | ||||||
|
|
||||||
| if (!VT.isFloatingPoint()) | ||||||
| V = DAG.getNode(ISD::ANY_EXTEND, DL, Subtarget.getXLenVT(), V); | ||||||
| Vec = DAG.getNode(Opcode, DL, ContainerVT, DAG.getUNDEF(ContainerVT), Vec, | ||||||
| V, Mask, VL); | ||||||
| } | ||||||
| if (UndefCount) { | ||||||
| const SDValue Offset = DAG.getConstant(UndefCount, DL, Subtarget.getXLenVT()); | ||||||
| Vec = getVSlide(ContainerVT, DAG.getUNDEF(ContainerVT), Vec, Offset, Mask, | ||||||
| VL); | ||||||
| } | ||||||
| return convertFromScalableVector(VT, Vec, DAG, Subtarget); | ||||||
| } | ||||||
|
|
||||||
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Nit, can we get away with just copying the iterator and not the storage? Does something like this work
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.
Good question and I guess it's related to your
llvm::reversecomment earlier: we can keep everything iterator (ranges) if we can write the following code:but unfortunately I don't think that would be possible without some iterator type adaption, given
llvm::reversehaving a different type thanop_values().