From 62449823476bf71c2510c174e387c9c99d219722 Mon Sep 17 00:00:00 2001 From: eopXD Date: Thu, 5 Jan 2023 03:37:37 -0800 Subject: [PATCH] [1/15][Clang][RISCV][NFC] Extract common utility to RISCVVIntrinsicUtils This is the 1st commit of a patch-set that aims to change the default policy for RVV intrinsics from TAMU to TAMA. The patch-set work towards the simplification proposal [0] of Nick Knight. After this patch-set, all intrinsics operates under a general assumption that the policy behavior is agnostic. You may find that most of the patches are NFC patches. They subtly remove implicit assumptions that entangles the codebase, making the singular patch that contains functional change clear and obvious. In [2/15], The attribute `Policy::IsPolicyNone` may give the mis-perception that an RVV intrinsic may operate without any policy. However this is not the case because the policy CSR-s (`vta` and `vma`) always affect an RVV instruction's behavior, except that some instructions have policy always set as agnostic (e.g. instructions with a mask destination register is always tail agnostic). Next, to perform the change from TAMU to TAMA, we need to first remove `Policy::PolicyType::Omit`. [4/15] ~ [12/15] removes it with NFC patches step by step. Without the patches, directly applying [14/15] to the existing codebase will not work because there will be complicated logics that are scattered in places that is hard to maintain. [1/15], [3/15] are not related to the main goal of this patch-set, they were clean-up along the way as I was going through the codebase. [13/15] is a clean-up that was an oversight in D141198. Finally, [14/15] performs the functional change this patch-set aims for. The default policy is changed from TAMU to TAMA. This affects the masked version of the intrinsics without a policy suffix. The masked-off operand is removed. Due to the removal, masked version of `vid` and `viota` intrinsics are no longer available for overloading. [15/15] is a final commit to set data members of `Policy` as constants. Through the refactoring the class `Policy` is now correct-by-construction. The next patch-set will be to remove redundant intrinsics with a policy suffix `_ta` and `_tama` intrinsics are redundant and will be removed. Other policy suffix will be renamed to adapt to the general assumption that policy is generally agnostic. [0] https://gist.github.com/nick-knight/6cb0b74b351a25323dfb1821d3a269b9 Pull Request: riscv-non-isa/rvv-intrinsic-doc#186 Reviewed By: craig.topper, kito-cheng Differential Revision: https://reviews.llvm.org/D141573 --- clang/include/clang/Support/RISCVVIntrinsicUtils.h | 2 ++ clang/lib/Sema/SemaRISCVVectorLookup.cpp | 10 +++------- clang/lib/Support/RISCVVIntrinsicUtils.cpp | 6 ++++++ clang/utils/TableGen/RISCVVEmitter.cpp | 8 ++------ 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/clang/include/clang/Support/RISCVVIntrinsicUtils.h b/clang/include/clang/Support/RISCVVIntrinsicUtils.h index 85323c0..a0c72d9 100644 --- a/clang/include/clang/Support/RISCVVIntrinsicUtils.h +++ b/clang/include/clang/Support/RISCVVIntrinsicUtils.h @@ -472,6 +472,8 @@ public: bool IsMasked, bool HasMaskedOffOperand, bool HasVL, unsigned NF, PolicyScheme DefaultScheme, Policy PolicyAttrs); + + static llvm::SmallVector getSupportedUnMaskedPolicies(); static llvm::SmallVector getSupportedMaskedPolicies(bool HasTailPolicy, bool HasMaskPolicy); diff --git a/clang/lib/Sema/SemaRISCVVectorLookup.cpp b/clang/lib/Sema/SemaRISCVVectorLookup.cpp index 3f6ea67..8dab399 100644 --- a/clang/lib/Sema/SemaRISCVVectorLookup.cpp +++ b/clang/lib/Sema/SemaRISCVVectorLookup.cpp @@ -205,13 +205,9 @@ void RISCVIntrinsicManagerImpl::InitIntrinsicList() { bool UnMaskedHasPolicy = UnMaskedPolicyScheme != PolicyScheme::SchemeNone; bool MaskedHasPolicy = MaskedPolicyScheme != PolicyScheme::SchemeNone; - // If unmasked builtin supports policy, they should be TU or TA. - llvm::SmallVector SupportedUnMaskedPolicies; - SupportedUnMaskedPolicies.emplace_back(Policy( - Policy::PolicyType::Undisturbed, Policy::PolicyType::Omit)); // TU - SupportedUnMaskedPolicies.emplace_back( - Policy(Policy::PolicyType::Agnostic, Policy::PolicyType::Omit)); // TA - llvm::SmallVector SupportedMaskedPolicies = + SmallVector SupportedUnMaskedPolicies = + RVVIntrinsic::getSupportedUnMaskedPolicies(); + SmallVector SupportedMaskedPolicies = RVVIntrinsic::getSupportedMaskedPolicies(Record.HasTailPolicy, Record.HasMaskPolicy); diff --git a/clang/lib/Support/RISCVVIntrinsicUtils.cpp b/clang/lib/Support/RISCVVIntrinsicUtils.cpp index e15dd89..5022426 100644 --- a/clang/lib/Support/RISCVVIntrinsicUtils.cpp +++ b/clang/lib/Support/RISCVVIntrinsicUtils.cpp @@ -974,6 +974,12 @@ llvm::SmallVector RVVIntrinsic::computeBuiltinTypes( return NewPrototype; } +llvm::SmallVector RVVIntrinsic::getSupportedUnMaskedPolicies() { + return { + Policy(Policy::PolicyType::Undisturbed, Policy::PolicyType::Omit), // TU + Policy(Policy::PolicyType::Agnostic, Policy::PolicyType::Omit)}; // TA +} + llvm::SmallVector RVVIntrinsic::getSupportedMaskedPolicies(bool HasTailPolicy, bool HasMaskPolicy) { diff --git a/clang/utils/TableGen/RISCVVEmitter.cpp b/clang/utils/TableGen/RISCVVEmitter.cpp index 7c58b7d..eb9ec3e 100644 --- a/clang/utils/TableGen/RISCVVEmitter.cpp +++ b/clang/utils/TableGen/RISCVVEmitter.cpp @@ -528,12 +528,8 @@ void RVVEmitter::createRVVIntrinsics( StringRef MaskedIRName = R->getValueAsString("MaskedIRName"); unsigned NF = R->getValueAsInt("NF"); - // If unmasked builtin supports policy, they should be TU or TA. - llvm::SmallVector SupportedUnMaskedPolicies; - SupportedUnMaskedPolicies.emplace_back(Policy( - Policy::PolicyType::Undisturbed, Policy::PolicyType::Omit)); // TU - SupportedUnMaskedPolicies.emplace_back( - Policy(Policy::PolicyType::Agnostic, Policy::PolicyType::Omit)); // TA + SmallVector SupportedUnMaskedPolicies = + RVVIntrinsic::getSupportedUnMaskedPolicies(); SmallVector SupportedMaskedPolicies = RVVIntrinsic::getSupportedMaskedPolicies(HasTailPolicy, HasMaskPolicy); -- 2.7.4