Skip to content
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.

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

[RISCV][clang] Optimize memory usage of intrinsic lookup table #77487

Merged
merged 3 commits into from
Jan 26, 2024

Conversation

4vtomat
Copy link
Member

@4vtomat 4vtomat commented Jan 9, 2024

This patch optimize:

  1. Reduce string size of RVVIntrinsicDef.
  2. Reduce the type size of the index of intrinsics.

I use valgrind --tool=massif to analyze a simple program:

#include <riscv_vector.h>
vint32m1_t test(vint32m1_t v1, vint32m1_t v2, size_t vl) {
  return __riscv_vadd(v1, v2, vl);
}

and before optimization, the peak memory usage is 15.68MB,
after optimization, the peak memory usage is 13.69MB.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 9, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 9, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Brandon Wu (4vtomat)

Changes

This patch optimize:

  1. Reduce string size of RVVIntrinsicDef.
  2. Reduce the type size of the index of intrinsics.

I use valgrind --tool=massif to analyze a simple program:

#include &lt;riscv_vector.h&gt;
vint32m1_t test(vint32m1_t v1, vint32m1_t v2, size_t vl) {
  return __riscv_vadd(v1, v2, vl);
}

and before optimization, the peak memory usage is 15.68MB,
after optimization, the peak memory usage is 13.69MB.


Full diff: https://github.com/llvm/llvm-project/pull/77487.diff

3 Files Affected:

  • (modified) clang/include/clang/Support/RISCVVIntrinsicUtils.h (+4-2)
  • (modified) clang/lib/Sema/SemaRISCVVectorLookup.cpp (+7-6)
  • (modified) clang/lib/Support/RISCVVIntrinsicUtils.cpp (-5)
diff --git a/clang/include/clang/Support/RISCVVIntrinsicUtils.h b/clang/include/clang/Support/RISCVVIntrinsicUtils.h
index c525d3443331e0..7e20f022c28b55 100644
--- a/clang/include/clang/Support/RISCVVIntrinsicUtils.h
+++ b/clang/include/clang/Support/RISCVVIntrinsicUtils.h
@@ -416,8 +416,10 @@ class RVVIntrinsic {
   RVVTypePtr getOutputType() const { return OutputType; }
   const RVVTypes &getInputTypes() const { return InputTypes; }
   llvm::StringRef getBuiltinName() const { return BuiltinName; }
-  llvm::StringRef getName() const { return Name; }
-  llvm::StringRef getOverloadedName() const { return OverloadedName; }
+  llvm::StringRef getName() const { return "__riscv_" + Name; }
+  llvm::StringRef getOverloadedName() const {
+    return "__riscv_" + OverloadedName;
+  }
   bool hasMaskedOffOperand() const { return HasMaskedOffOperand; }
   bool hasVL() const { return HasVL; }
   bool hasPolicy() const { return Scheme != PolicyScheme::SchemeNone; }
diff --git a/clang/lib/Sema/SemaRISCVVectorLookup.cpp b/clang/lib/Sema/SemaRISCVVectorLookup.cpp
index 3ed3e619544189..e9523871e9cd1f 100644
--- a/clang/lib/Sema/SemaRISCVVectorLookup.cpp
+++ b/clang/lib/Sema/SemaRISCVVectorLookup.cpp
@@ -43,7 +43,7 @@ struct RVVIntrinsicDef {
 
 struct RVVOverloadIntrinsicDef {
   // Indexes of RISCVIntrinsicManagerImpl::IntrinsicList.
-  SmallVector<uint32_t, 8> Indexes;
+  SmallVector<uint16_t, 8> Indexes;
 };
 
 } // namespace
@@ -162,7 +162,7 @@ class RISCVIntrinsicManagerImpl : public sema::RISCVIntrinsicManager {
   // List of all RVV intrinsic.
   std::vector<RVVIntrinsicDef> IntrinsicList;
   // Mapping function name to index of IntrinsicList.
-  StringMap<uint32_t> Intrinsics;
+  StringMap<uint16_t> Intrinsics;
   // Mapping function name to RVVOverloadIntrinsicDef.
   StringMap<RVVOverloadIntrinsicDef> OverloadIntrinsics;
 
@@ -380,14 +380,14 @@ void RISCVIntrinsicManagerImpl::InitRVVIntrinsic(
     OverloadedName += "_" + OverloadedSuffixStr.str();
 
   // clang built-in function name, e.g. __builtin_rvv_vadd.
-  std::string BuiltinName = "__builtin_rvv_" + std::string(Record.Name);
+  std::string BuiltinName = std::string(Record.Name);
 
   RVVIntrinsic::updateNamesAndPolicy(IsMasked, HasPolicy, Name, BuiltinName,
                                      OverloadedName, PolicyAttrs,
                                      Record.HasFRMRoundModeOp);
 
   // Put into IntrinsicList.
-  uint32_t Index = IntrinsicList.size();
+  uint16_t Index = IntrinsicList.size();
   IntrinsicList.push_back({BuiltinName, Signature});
 
   // Creating mapping to Intrinsics.
@@ -452,7 +452,8 @@ void RISCVIntrinsicManagerImpl::CreateRVVIntrinsicDecl(LookupResult &LR,
     RVVIntrinsicDecl->addAttr(OverloadableAttr::CreateImplicit(Context));
 
   // Setup alias to __builtin_rvv_*
-  IdentifierInfo &IntrinsicII = PP.getIdentifierTable().get(IDef.BuiltinName);
+  IdentifierInfo &IntrinsicII =
+      PP.getIdentifierTable().get("__builtin_rvv_" + IDef.BuiltinName);
   RVVIntrinsicDecl->addAttr(
       BuiltinAliasAttr::CreateImplicit(S.Context, &IntrinsicII));
 
@@ -463,7 +464,7 @@ void RISCVIntrinsicManagerImpl::CreateRVVIntrinsicDecl(LookupResult &LR,
 bool RISCVIntrinsicManagerImpl::CreateIntrinsicIfFound(LookupResult &LR,
                                                        IdentifierInfo *II,
                                                        Preprocessor &PP) {
-  StringRef Name = II->getName();
+  StringRef Name = II->getName().substr(8);
 
   // Lookup the function name from the overload intrinsics first.
   auto OvIItr = OverloadIntrinsics.find(Name);
diff --git a/clang/lib/Support/RISCVVIntrinsicUtils.cpp b/clang/lib/Support/RISCVVIntrinsicUtils.cpp
index 2de977a3dc720b..7d2a2d7e826f9c 100644
--- a/clang/lib/Support/RISCVVIntrinsicUtils.cpp
+++ b/clang/lib/Support/RISCVVIntrinsicUtils.cpp
@@ -1150,11 +1150,6 @@ void RVVIntrinsic::updateNamesAndPolicy(
     OverloadedName += suffix;
   };
 
-  // This follows the naming guideline under riscv-c-api-doc to add the
-  // `__riscv_` suffix for all RVV intrinsics.
-  Name = "__riscv_" + Name;
-  OverloadedName = "__riscv_" + OverloadedName;
-
   if (HasFRMRoundModeOp) {
     Name += "_rm";
     BuiltinName += "_rm";

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 9, 2024

@llvm/pr-subscribers-clang

Author: Brandon Wu (4vtomat)

Changes

This patch optimize:

  1. Reduce string size of RVVIntrinsicDef.
  2. Reduce the type size of the index of intrinsics.

I use valgrind --tool=massif to analyze a simple program:

#include &lt;riscv_vector.h&gt;
vint32m1_t test(vint32m1_t v1, vint32m1_t v2, size_t vl) {
  return __riscv_vadd(v1, v2, vl);
}

and before optimization, the peak memory usage is 15.68MB,
after optimization, the peak memory usage is 13.69MB.


Full diff: https://github.com/llvm/llvm-project/pull/77487.diff

3 Files Affected:

  • (modified) clang/include/clang/Support/RISCVVIntrinsicUtils.h (+4-2)
  • (modified) clang/lib/Sema/SemaRISCVVectorLookup.cpp (+7-6)
  • (modified) clang/lib/Support/RISCVVIntrinsicUtils.cpp (-5)
diff --git a/clang/include/clang/Support/RISCVVIntrinsicUtils.h b/clang/include/clang/Support/RISCVVIntrinsicUtils.h
index c525d3443331e0..7e20f022c28b55 100644
--- a/clang/include/clang/Support/RISCVVIntrinsicUtils.h
+++ b/clang/include/clang/Support/RISCVVIntrinsicUtils.h
@@ -416,8 +416,10 @@ class RVVIntrinsic {
   RVVTypePtr getOutputType() const { return OutputType; }
   const RVVTypes &getInputTypes() const { return InputTypes; }
   llvm::StringRef getBuiltinName() const { return BuiltinName; }
-  llvm::StringRef getName() const { return Name; }
-  llvm::StringRef getOverloadedName() const { return OverloadedName; }
+  llvm::StringRef getName() const { return "__riscv_" + Name; }
+  llvm::StringRef getOverloadedName() const {
+    return "__riscv_" + OverloadedName;
+  }
   bool hasMaskedOffOperand() const { return HasMaskedOffOperand; }
   bool hasVL() const { return HasVL; }
   bool hasPolicy() const { return Scheme != PolicyScheme::SchemeNone; }
diff --git a/clang/lib/Sema/SemaRISCVVectorLookup.cpp b/clang/lib/Sema/SemaRISCVVectorLookup.cpp
index 3ed3e619544189..e9523871e9cd1f 100644
--- a/clang/lib/Sema/SemaRISCVVectorLookup.cpp
+++ b/clang/lib/Sema/SemaRISCVVectorLookup.cpp
@@ -43,7 +43,7 @@ struct RVVIntrinsicDef {
 
 struct RVVOverloadIntrinsicDef {
   // Indexes of RISCVIntrinsicManagerImpl::IntrinsicList.
-  SmallVector<uint32_t, 8> Indexes;
+  SmallVector<uint16_t, 8> Indexes;
 };
 
 } // namespace
@@ -162,7 +162,7 @@ class RISCVIntrinsicManagerImpl : public sema::RISCVIntrinsicManager {
   // List of all RVV intrinsic.
   std::vector<RVVIntrinsicDef> IntrinsicList;
   // Mapping function name to index of IntrinsicList.
-  StringMap<uint32_t> Intrinsics;
+  StringMap<uint16_t> Intrinsics;
   // Mapping function name to RVVOverloadIntrinsicDef.
   StringMap<RVVOverloadIntrinsicDef> OverloadIntrinsics;
 
@@ -380,14 +380,14 @@ void RISCVIntrinsicManagerImpl::InitRVVIntrinsic(
     OverloadedName += "_" + OverloadedSuffixStr.str();
 
   // clang built-in function name, e.g. __builtin_rvv_vadd.
-  std::string BuiltinName = "__builtin_rvv_" + std::string(Record.Name);
+  std::string BuiltinName = std::string(Record.Name);
 
   RVVIntrinsic::updateNamesAndPolicy(IsMasked, HasPolicy, Name, BuiltinName,
                                      OverloadedName, PolicyAttrs,
                                      Record.HasFRMRoundModeOp);
 
   // Put into IntrinsicList.
-  uint32_t Index = IntrinsicList.size();
+  uint16_t Index = IntrinsicList.size();
   IntrinsicList.push_back({BuiltinName, Signature});
 
   // Creating mapping to Intrinsics.
@@ -452,7 +452,8 @@ void RISCVIntrinsicManagerImpl::CreateRVVIntrinsicDecl(LookupResult &LR,
     RVVIntrinsicDecl->addAttr(OverloadableAttr::CreateImplicit(Context));
 
   // Setup alias to __builtin_rvv_*
-  IdentifierInfo &IntrinsicII = PP.getIdentifierTable().get(IDef.BuiltinName);
+  IdentifierInfo &IntrinsicII =
+      PP.getIdentifierTable().get("__builtin_rvv_" + IDef.BuiltinName);
   RVVIntrinsicDecl->addAttr(
       BuiltinAliasAttr::CreateImplicit(S.Context, &IntrinsicII));
 
@@ -463,7 +464,7 @@ void RISCVIntrinsicManagerImpl::CreateRVVIntrinsicDecl(LookupResult &LR,
 bool RISCVIntrinsicManagerImpl::CreateIntrinsicIfFound(LookupResult &LR,
                                                        IdentifierInfo *II,
                                                        Preprocessor &PP) {
-  StringRef Name = II->getName();
+  StringRef Name = II->getName().substr(8);
 
   // Lookup the function name from the overload intrinsics first.
   auto OvIItr = OverloadIntrinsics.find(Name);
diff --git a/clang/lib/Support/RISCVVIntrinsicUtils.cpp b/clang/lib/Support/RISCVVIntrinsicUtils.cpp
index 2de977a3dc720b..7d2a2d7e826f9c 100644
--- a/clang/lib/Support/RISCVVIntrinsicUtils.cpp
+++ b/clang/lib/Support/RISCVVIntrinsicUtils.cpp
@@ -1150,11 +1150,6 @@ void RVVIntrinsic::updateNamesAndPolicy(
     OverloadedName += suffix;
   };
 
-  // This follows the naming guideline under riscv-c-api-doc to add the
-  // `__riscv_` suffix for all RVV intrinsics.
-  Name = "__riscv_" + Name;
-  OverloadedName = "__riscv_" + OverloadedName;
-
   if (HasFRMRoundModeOp) {
     Name += "_rm";
     BuiltinName += "_rm";

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

This patch optimize:
  1. Reduce string size of RVVIntrinsicDef.
  2. Reduce the type size of the index of intrinsics.

I use valgrind --tool=massif to analyze a simple program:
```
#include <riscv_vector.h>
vint32m1_t test(vint32m1_t v1, vint32m1_t v2, size_t vl) {
  return __riscv_vadd(v1, v2, vl);
}
```
and before optimization, the peak memory usage is 15.68MB,
after optimization, the peak memory usage is 13.69MB.
@4vtomat 4vtomat force-pushed the opt_mem_usage_in_SemaRISCVVectorLookup_cpp branch from 8d40522 to f8013d5 Compare January 26, 2024 03:16
@4vtomat 4vtomat merged commit e0092ea into llvm:main Jan 26, 2024
3 of 4 checks passed
@4vtomat 4vtomat deleted the opt_mem_usage_in_SemaRISCVVectorLookup_cpp branch January 26, 2024 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants