[llvm] [NVPTX] Lower LLVM masked vector loads and stores to PTX (PR #159387)
Drew Kersnar via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 27 14:43:46 PDT 2025
================
@@ -186,12 +186,16 @@ class ARMTTIImpl final : public BasicTTIImplBase<ARMTTIImpl> {
bool isProfitableLSRChainElement(Instruction *I) const override;
- bool isLegalMaskedLoad(Type *DataTy, Align Alignment,
- unsigned AddressSpace) const override;
-
- bool isLegalMaskedStore(Type *DataTy, Align Alignment,
- unsigned AddressSpace) const override {
- return isLegalMaskedLoad(DataTy, Alignment, AddressSpace);
+ bool
+ isLegalMaskedLoad(Type *DataTy, Align Alignment, unsigned AddressSpace,
+ TTI::MaskKind MaskKind =
+ TTI::MaskKind::VariableOrConstantMask) const override;
+
+ bool
+ isLegalMaskedStore(Type *DataTy, Align Alignment, unsigned AddressSpace,
+ TTI::MaskKind MaskKind =
+ TTI::MaskKind::VariableOrConstantMask) const override {
+ return isLegalMaskedLoad(DataTy, Alignment, AddressSpace, MaskKind);
}
----------------
dakersnar wrote:
> Right now the new argument is ignored by the existing implementations and that may result in them giving the wrong answer if someone looks specifically for constant mask (or specifically variable mask, if we add another enum for that).
I think it wouldn't be the _wrong_ answer, as I think the assumption is that every target that has already overridden this API does not care about the constantness of the mask when determining legality. Call sites that pass ConstantMask are not "looking specifically for constant mask", they are adding the additional information that the mask they are checking is constant, which for NVPTX is a needed assertion in order to flip the answer from `false` to `true`.
> You could avoid having to change all target overrides by making it a separate overload with the default implementation calling the old API if MaskKind = VariableOrConstantMask and erroring out or returning false otherwise.
I think this might cause some confusion at the call sites as to which version of the API to call, right? The correct answer would be to call the one that includes the MaskKind parameter if 1) you can prove that a mask is constant and 2) if the targets that care about constantness matter enough to you to spend the compile time to do the check. This question is already present with the current "default argument" approach but I think it helps reduce confusion to have it on one API.
Totally open to being wrong on this though, I wrestled a lot with the shape of this API change and am not at all confident I'm correct.
https://github.com/llvm/llvm-project/pull/159387
More information about the llvm-commits
mailing list