[Clang][RISCV] Fix RISC-V vector / SiFive intrinsic inclusion in SemaLookup

[eopXD]

Summary

The existing code assumes that both DeclareRISCVVBuiltins and
DeclareRISCVSiFiveVectorBuiltins are set when coming into the if-statement
under SemaLookup.cpp.

This is not the case and causes issue #63571.

This patch resolves the issue.

[eopXD]

Add test case of C++ compilation rvv-intrinsics-handcrafted/xsfvcp.cpp

[kito-cheng]

LGTM, thanks for quick fix!

[eopXD]

Bump CI.

[4vtomat]

LGTM, thanks~

[jrtc27]

clang/lib/Sema/SemaRISCVVectorLookup.cpp

201

As far as I can tell this refactoring was entirely gratuitous, and perhaps a regression given the function is only ever used by InitIntrinsicList and thus can be hidden as a lambda?

clang/include/clang/Sema/RISCVIntrinsicManager.h

 
 namespace sema {
 class RISCVIntrinsicManager {
 public:
   enum class IntrinsicKind : uint8_t { RVV, SIFIVE_VECTOR };
 
   virtual ~RISCVIntrinsicManager() = default;
 
+  virtual void InitIntrinsicList() = 0;
+
   // Create RISC-V intrinsic and insert into symbol table and return true if
   // found, otherwise return false.
   virtual bool CreateIntrinsicIfFound(LookupResult &LR, IdentifierInfo *II,
                                       Preprocessor &PP) = 0;
 };
 } // end namespace sema
 } // end namespace clang
 
 #endif

clang/lib/Sema/SemaLookup.cpp

if (II) {
           return true;
         }
       }
 
       if (DeclareRISCVVBuiltins || DeclareRISCVSiFiveVectorBuiltins) {
         if (!RVIntrinsicManager)
           RVIntrinsicManager = CreateRISCVIntrinsicManager(*this);
 
+        RVIntrinsicManager->InitIntrinsicList();
+
         if (RVIntrinsicManager->CreateIntrinsicIfFound(R, II, PP))
           return true;
       }
 
       // If this is a builtin on this (or all) targets, create the decl.
       if (unsigned BuiltinID = II->getBuiltinID()) {
         // In C++ and OpenCL (spec v1.2 s6.9.f), we don't have any predefined
         // library functions like 'malloc'. Instead, we'll just error.

clang/lib/Sema/SemaRISCVVectorLookup.cpp

 }
 
 namespace {
 class RISCVIntrinsicManagerImpl : public sema::RISCVIntrinsicManager {
 private:
   Sema &S;
   ASTContext &Context;
   RVVTypeCache TypeCache;
+  bool ConstructedRISCVVBuiltins;
+  bool ConstructedRISCVSiFiveVectorBuiltins;
 
   // List of all RVV intrinsic.
   std::vector<RVVIntrinsicDef> IntrinsicList;
   // Mapping function name to index of IntrinsicList.
   StringMap<size_t> Intrinsics;
   // Mapping function name to RVVOverloadIntrinsicDef.
   StringMap<RVVOverloadIntrinsicDef> OverloadIntrinsics;
 
-  // Create IntrinsicList
-  void InitIntrinsicList();
 
   // Create RVVIntrinsicDef.
   void InitRVVIntrinsic(const RVVIntrinsicRecord &Record, StringRef SuffixStr,
                         StringRef OverloadedSuffixStr, bool IsMask,
                         RVVTypes &Types, bool HasPolicy, Policy PolicyAttrs);
 
   // Create FunctionDecl for a vector intrinsic.
   void CreateRVVIntrinsicDecl(LookupResult &LR, IdentifierInfo *II,
                               Preprocessor &PP, unsigned Index,
                               bool IsOverload);
 
+  void ConstructRVVIntrinsics(ArrayRef<RVVIntrinsicRecord> Recs,
+                              IntrinsicKind K);
+
 public:
   RISCVIntrinsicManagerImpl(clang::Sema &S) : S(S), Context(S.Context) {
-    InitIntrinsicList();
+    ConstructedRISCVVBuiltins = false;
+    ConstructedRISCVSiFiveVectorBuiltins = false;
   }
 
+  // Initialize IntrinsicList
+  void InitIntrinsicList() override;
+
   // Create RISC-V vector intrinsic and insert into symbol table if found, and
   // return true, otherwise return false.
   bool CreateIntrinsicIfFound(LookupResult &LR, IdentifierInfo *II,
                               Preprocessor &PP) override;
 };
 } // namespace
 
-void RISCVIntrinsicManagerImpl::InitIntrinsicList() {
+void RISCVIntrinsicManagerImpl::ConstructRVVIntrinsics(
+    ArrayRef<RVVIntrinsicRecord> Recs, IntrinsicKind K) {
   const TargetInfo &TI = Context.getTargetInfo();
   bool HasRV64 = TI.hasFeature("64bit");
   bool HasFullMultiply = TI.hasFeature("v");
-
-  auto ConstructRVVIntrinsics = [&](ArrayRef<RVVIntrinsicRecord> Recs,
-                                    IntrinsicKind K) {
-    // Construction of RVVIntrinsicRecords need to sync with createRVVIntrinsics
-    // in RISCVVEmitter.cpp.
-    for (auto &Record : Recs) {
-      // Create Intrinsics for each type and LMUL.
-      BasicType BaseType = BasicType::Unknown;
-      ArrayRef<PrototypeDescriptor> BasicProtoSeq =
-          ProtoSeq2ArrayRef(K, Record.PrototypeIndex, Record.PrototypeLength);
-      ArrayRef<PrototypeDescriptor> SuffixProto =
-          ProtoSeq2ArrayRef(K, Record.SuffixIndex, Record.SuffixLength);
-      ArrayRef<PrototypeDescriptor> OverloadedSuffixProto = ProtoSeq2ArrayRef(
-          K, Record.OverloadedSuffixIndex, Record.OverloadedSuffixSize);
 
-      PolicyScheme UnMaskedPolicyScheme =
-          static_cast<PolicyScheme>(Record.UnMaskedPolicyScheme);
-      PolicyScheme MaskedPolicyScheme =
-          static_cast<PolicyScheme>(Record.MaskedPolicyScheme);
 
-      const Policy DefaultPolicy;
 
-      llvm::SmallVector<PrototypeDescriptor> ProtoSeq =
-          RVVIntrinsic::computeBuiltinTypes(
-              BasicProtoSeq, /*IsMasked=*/false,
-              /*HasMaskedOffOperand=*/false, Record.HasVL, Record.NF,
-              UnMaskedPolicyScheme, DefaultPolicy, Record.IsTuple);
 
-      llvm::SmallVector<PrototypeDescriptor> ProtoMaskSeq =
-          RVVIntrinsic::computeBuiltinTypes(
-              BasicProtoSeq, /*IsMasked=*/true, Record.HasMaskedOffOperand,
-              Record.HasVL, Record.NF, MaskedPolicyScheme, DefaultPolicy,
-              Record.IsTuple);
 
-      bool UnMaskedHasPolicy = UnMaskedPolicyScheme != PolicyScheme::SchemeNone;
-      bool MaskedHasPolicy = MaskedPolicyScheme != PolicyScheme::SchemeNone;
-      SmallVector<Policy> SupportedUnMaskedPolicies =
-          RVVIntrinsic::getSupportedUnMaskedPolicies();
-      SmallVector<Policy> SupportedMaskedPolicies =
-          RVVIntrinsic::getSupportedMaskedPolicies(Record.HasTailPolicy,
-                                                   Record.HasMaskPolicy);
 
-      for (unsigned int TypeRangeMaskShift = 0;
-           TypeRangeMaskShift <= static_cast<unsigned int>(BasicType::MaxOffset);
-           ++TypeRangeMaskShift) {
-        unsigned int BaseTypeI = 1 << TypeRangeMaskShift;
-        BaseType = static_cast<BasicType>(BaseTypeI);
 
-        if ((BaseTypeI & Record.TypeRangeMask) != BaseTypeI)
-          continue;
 
-        // Check requirement.
-        if (((Record.RequiredExtensions & RVV_REQ_RV64) == RVV_REQ_RV64) &&
-            !HasRV64)
-          continue;
 
-        if ((BaseType == BasicType::Int64) &&
-            ((Record.RequiredExtensions & RVV_REQ_FullMultiply) ==
-             RVV_REQ_FullMultiply) &&
-            !HasFullMultiply)
-          continue;
 
-        // Expanded with different LMUL.
-        for (int Log2LMUL = -3; Log2LMUL <= 3; Log2LMUL++) {
-          if (!(Record.Log2LMULMask & (1 << (Log2LMUL + 3))))
-            continue;
 
-          std::optional<RVVTypes> Types =
-              TypeCache.computeTypes(BaseType, Log2LMUL, Record.NF, ProtoSeq);
 
-          // Ignored to create new intrinsic if there are any illegal types.
-          if (!Types.has_value())
-            continue;
 
-          std::string SuffixStr = RVVIntrinsic::getSuffixStr(
-              TypeCache, BaseType, Log2LMUL, SuffixProto);
-          std::string OverloadedSuffixStr = RVVIntrinsic::getSuffixStr(
-              TypeCache, BaseType, Log2LMUL, OverloadedSuffixProto);
 
-          // Create non-masked intrinsic.
-          InitRVVIntrinsic(Record, SuffixStr, OverloadedSuffixStr, false, *Types,
-                           UnMaskedHasPolicy, DefaultPolicy);
 
-          // Create non-masked policy intrinsic.
-          if (Record.UnMaskedPolicyScheme != PolicyScheme::SchemeNone) {
-            for (auto P : SupportedUnMaskedPolicies) {
-              llvm::SmallVector<PrototypeDescriptor> PolicyPrototype =
-                  RVVIntrinsic::computeBuiltinTypes(
-                      BasicProtoSeq, /*IsMasked=*/false,
-                      /*HasMaskedOffOperand=*/false, Record.HasVL, Record.NF,
-                      UnMaskedPolicyScheme, P, Record.IsTuple);
-              std::optional<RVVTypes> PolicyTypes = TypeCache.computeTypes(
-                  BaseType, Log2LMUL, Record.NF, PolicyPrototype);
-              InitRVVIntrinsic(Record, SuffixStr, OverloadedSuffixStr,
-                               /*IsMask=*/false, *PolicyTypes, UnMaskedHasPolicy,
-                               P);
-            }
-          }
-          if (!Record.HasMasked)
-            continue;
-          // Create masked intrinsic.
-          std::optional<RVVTypes> MaskTypes =
-              TypeCache.computeTypes(BaseType, Log2LMUL, Record.NF, ProtoMaskSeq);
-          InitRVVIntrinsic(Record, SuffixStr, OverloadedSuffixStr, true,
-                           *MaskTypes, MaskedHasPolicy, DefaultPolicy);
-          if (Record.MaskedPolicyScheme == PolicyScheme::SchemeNone)
-            continue;
-          // Create masked policy intrinsic.
-          for (auto P : SupportedMaskedPolicies) {
-            llvm::SmallVector<PrototypeDescriptor> PolicyPrototype =
-                RVVIntrinsic::computeBuiltinTypes(
-                    BasicProtoSeq, /*IsMasked=*/true,
-                    Record.HasMaskedOffOperand, Record.HasVL, Record.NF,
-                    MaskedPolicyScheme, P, Record.IsTuple);
-            std::optional<RVVTypes> PolicyTypes = TypeCache.computeTypes(
-                BaseType, Log2LMUL, Record.NF, PolicyPrototype);
-            InitRVVIntrinsic(Record, SuffixStr, OverloadedSuffixStr,
-                             /*IsMask=*/true, *PolicyTypes, MaskedHasPolicy, P);
-          }
-        } // End for different LMUL
-      } // End for different TypeRange
-    }
-  };
-  if (S.DeclareRISCVVBuiltins)
+                  BasicProtoSeq, /*IsMasked=*/true, Record.HasMaskedOffOperand,
+                  Record.HasVL, Record.NF, MaskedPolicyScheme, P,
+                  Record.IsTuple);
+    }   // End for different TypeRange
+}
+
+void RISCVIntrinsicManagerImpl::InitIntrinsicList() {
+
+  if (S.DeclareRISCVVBuiltins && !ConstructedRISCVVBuiltins) {
+    ConstructedRISCVVBuiltins = true;
     ConstructRVVIntrinsics(RVVIntrinsicRecords,
                            IntrinsicKind::RVV);
-  if (S.DeclareRISCVSiFiveVectorBuiltins)
+  }
+  if (S.DeclareRISCVSiFiveVectorBuiltins &&
+      !ConstructedRISCVSiFiveVectorBuiltins) {
+    ConstructedRISCVSiFiveVectorBuiltins = true;
     ConstructRVVIntrinsics(RVSiFiveVectorIntrinsicRecords,
                            IntrinsicKind::SIFIVE_VECTOR);
   }
+}
 
 // Compute name and signatures for intrinsic with practical types.
 void RISCVIntrinsicManagerImpl::InitRVVIntrinsic(
     const RVVIntrinsicRecord &Record, StringRef SuffixStr,
     StringRef OverloadedSuffixStr, bool IsMasked, RVVTypes &Signature,
     bool HasPolicy, Policy PolicyAttrs) {
   // Function name, e.g. vadd_vv_i32m1.
   std::string Name = Record.Name;

clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/xsfvcp.cpp

+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d \
+// RUN:   -target-feature +v  \
+// RUN:   -target-feature +xsfvcp \
+// RUN:   -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+#include <sifive_vector.h>
+
+#define p27_26 (0b11)
+#define p11_7  (0b11111)
+
+void test_sf_vc_xv_se_u64m1(vuint64m1_t vs2, uint64_t rs1, size_t vl) {
+  __riscv_sf_vc_xv_se_u64m1(p27_26, p11_7, vs2, rs1, vl);
+}