Skip to content

[RISCV] Support llvm.masked.compressstore intrinsic #83457

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Mar 13, 2024
Merged

[RISCV] Support llvm.masked.compressstore intrinsic #83457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7034dc8
[RISCV] Support `llvm.masked.compressstore` intrinsic
nikolaypanchenko Feb 29, 2024
9354dad
Added alignment argument to TTI for compress/expand
nikolaypanchenko Feb 29, 2024
4b521bb
Properly check if scalar type is legal for RVV
nikolaypanchenko Mar 1, 2024
276abb7
rebase
nikolaypanchenko Mar 6, 2024
f5fb36d
Addressed comments
nikolaypanchenko Mar 12, 2024
b90049a
Applied suggestions
nikolaypanchenko Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions llvm/lib/Target/RISCV/RISCVISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10466,6 +10466,7 @@ SDValue RISCVTargetLowering::lowerMaskedStore(SDValue Op,
SDValue BasePtr = MemSD->getBasePtr();
SDValue Val, Mask, VL;

bool IsCompressingStore = false;
if (const auto *VPStore = dyn_cast<VPStoreSDNode>(Op)) {
Val = VPStore->getValue();
Mask = VPStore->getMask();
Expand All @@ -10474,9 +10475,11 @@ SDValue RISCVTargetLowering::lowerMaskedStore(SDValue Op,
const auto *MStore = cast<MaskedStoreSDNode>(Op);
Val = MStore->getValue();
Mask = MStore->getMask();
IsCompressingStore = MStore->isCompressingStore();
}

bool IsUnmasked = ISD::isConstantSplatVectorAllOnes(Mask.getNode());
bool IsUnmasked =
ISD::isConstantSplatVectorAllOnes(Mask.getNode()) || IsCompressingStore;

MVT VT = Val.getSimpleValueType();
MVT XLenVT = Subtarget.getXLenVT();
Expand All @@ -10486,7 +10489,7 @@ SDValue RISCVTargetLowering::lowerMaskedStore(SDValue Op,
ContainerVT = getContainerForFixedLengthVector(VT);

Val = convertToScalableVector(ContainerVT, Val, DAG, Subtarget);
if (!IsUnmasked) {
if (!IsUnmasked || IsCompressingStore) {
MVT MaskVT = getMaskTypeFor(ContainerVT);
Mask = convertToScalableVector(MaskVT, Mask, DAG, Subtarget);
}
Expand All @@ -10495,6 +10498,15 @@ SDValue RISCVTargetLowering::lowerMaskedStore(SDValue Op,
if (!VL)
VL = getDefaultVLOps(VT, ContainerVT, DL, DAG, Subtarget).second;

if (IsCompressingStore) {
Val = DAG.getNode(ISD::INTRINSIC_WO_CHAIN, DL, ContainerVT,
DAG.getConstant(Intrinsic::riscv_vcompress, DL, XLenVT),
DAG.getUNDEF(ContainerVT), Val, Mask, VL);
VL =
DAG.getNode(RISCVISD::VCPOP_VL, DL, XLenVT, Mask,
getAllOnesMask(Mask.getSimpleValueType(), VL, DL, DAG), VL);
}

unsigned IntID =
IsUnmasked ? Intrinsic::riscv_vse : Intrinsic::riscv_vse_mask;
SmallVector<SDValue, 8> Ops{Chain, DAG.getTargetConstant(IntID, DL, XLenVT)};
Expand Down
10 changes: 10 additions & 0 deletions llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1620,3 +1620,13 @@ bool RISCVTTIImpl::isLSRCostLess(const TargetTransformInfo::LSRCost &C1,
C2.NumIVMuls, C2.NumBaseAdds,
C2.ScaleCost, C2.ImmCost, C2.SetupCost);
}

bool RISCVTTIImpl::isLegalMaskedCompressStore(Type *DataTy, Align Alignment) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to check whether the fixed vector type is legal like isLegalMaskedLoadStore().

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could also just use isLegalMaskedLoadStore() for compress/expandload like masked.load/store?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unlike x86, if store is done via separate instruction and requires masking, RVV's implementation does not require masking/predication due to tweaked VL.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The check being asked for is that fixed vectors are supported. Not that the specific type is legal. We still have a master disable for fixed vectors in the backend by setting -rvv-vector-bits-min=0.

    // Only support fixed vectors if we know the minimum vector size.
    if (DataTypeVT.isFixedLengthVector() && !ST->useRVVForFixedLengthVectors())
      return false;

auto *VTy = dyn_cast<VectorType>(DataTy);
if (!VTy || VTy->isScalableTy())
return false;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to check alignment unless you add misaligned support to RISCVISelLowering.cpp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I'll add isLegalMaskedLoadStore as @yetingk suggested.

if (!isLegalMaskedLoadStore(DataTy, Alignment))
return false;
return true;
}
2 changes: 2 additions & 0 deletions llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ class RISCVTTIImpl : public BasicTTIImplBase<RISCVTTIImpl> {
return TLI->isLegalStridedLoadStore(DataTypeVT, Alignment);
}

bool isLegalMaskedCompressStore(Type *DataTy, Align Alignment);

bool isVScaleKnownToBeAPowerOfTwo() const {
return TLI->isVScaleKnownToBeAPowerOfTwo();
}
Expand Down
Loading