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

[clang][Interp] Emit const references for Float arguments #79753

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

tbaederr
Copy link
Contributor

The Float print type is backed by the Floating class, which in turn uses APFloat, which might heap-allocate memory, so might be expensive to copy.

Add an 'AsRef' bit to the ArgType tablegen class, which defines whether we pass the argument around by copy or by reference.

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

llvmbot commented Jan 28, 2024

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

The Float print type is backed by the Floating class, which in turn uses APFloat, which might heap-allocate memory, so might be expensive to copy.

Add an 'AsRef' bit to the ArgType tablegen class, which defines whether we pass the argument around by copy or by reference.


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

2 Files Affected:

  • (modified) clang/lib/AST/Interp/Opcodes.td (+2-2)
  • (modified) clang/utils/TableGen/ClangOpcodesEmitter.cpp (+40-10)
diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
index 24747b6b98c163..6283e388d7a526 100644
--- a/clang/lib/AST/Interp/Opcodes.td
+++ b/clang/lib/AST/Interp/Opcodes.td
@@ -35,7 +35,7 @@ def FnPtr : Type;
 // Types transferred to the interpreter.
 //===----------------------------------------------------------------------===//
 
-class ArgType { string Name = ?; }
+class ArgType { string Name = ?; bit AsRef = false; }
 def ArgSint8 : ArgType { let Name = "int8_t"; }
 def ArgUint8 : ArgType { let Name = "uint8_t"; }
 def ArgSint16 : ArgType { let Name = "int16_t"; }
@@ -44,7 +44,7 @@ def ArgSint32 : ArgType { let Name = "int32_t"; }
 def ArgUint32 : ArgType { let Name = "uint32_t"; }
 def ArgSint64 : ArgType { let Name = "int64_t"; }
 def ArgUint64 : ArgType { let Name = "uint64_t"; }
-def ArgFloat : ArgType { let Name = "Floating"; }
+def ArgFloat : ArgType { let Name = "Floating"; let AsRef = true; }
 def ArgBool : ArgType { let Name = "bool"; }
 
 def ArgFunction : ArgType { let Name = "const Function *"; }
diff --git a/clang/utils/TableGen/ClangOpcodesEmitter.cpp b/clang/utils/TableGen/ClangOpcodesEmitter.cpp
index 02d5f9512d9051..1c41301ab3aeeb 100644
--- a/clang/utils/TableGen/ClangOpcodesEmitter.cpp
+++ b/clang/utils/TableGen/ClangOpcodesEmitter.cpp
@@ -126,9 +126,15 @@ void ClangOpcodesEmitter::EmitInterp(raw_ostream &OS, StringRef N,
 
               // Emit calls to read arguments.
               for (size_t I = 0, N = Args.size(); I < N; ++I) {
-                OS << "  auto V" << I;
+                const auto *Arg = Args[I];
+                bool AsRef = Arg->getValueAsBit("AsRef");
+
+                if (AsRef)
+                  OS << "  const auto &V" << I;
+                else
+                  OS << "  const auto V" << I;
                 OS << " = ";
-                OS << "ReadArg<" << Args[I]->getValueAsString("Name")
+                OS << "ReadArg<" << Arg->getValueAsString("Name")
                    << ">(S, PC);\n";
               }
 
@@ -192,8 +198,14 @@ void ClangOpcodesEmitter::EmitEmitter(raw_ostream &OS, StringRef N,
 
     // Emit the list of arguments.
     OS << "bool ByteCodeEmitter::emit" << ID << "(";
-    for (size_t I = 0, N = Args.size(); I < N; ++I)
-      OS << Args[I]->getValueAsString("Name") << " A" << I << ", ";
+    for (size_t I = 0, N = Args.size(); I < N; ++I) {
+      const auto *Arg = Args[I];
+      bool AsRef = Arg->getValueAsBit("AsRef");
+      auto Name = Arg->getValueAsString("Name");
+
+      OS << (AsRef ? "const " : " ") << Name << " " << (AsRef ? "&" : "") << "A"
+         << I << ", ";
+    }
     OS << "const SourceInfo &L) {\n";
 
     // Emit a call to write the opcodes.
@@ -218,8 +230,14 @@ void ClangOpcodesEmitter::EmitProto(raw_ostream &OS, StringRef N,
   auto Args = R->getValueAsListOfDefs("Args");
   Enumerate(R, N, [&OS, &Args](ArrayRef<const Record *> TS, const Twine &ID) {
     OS << "bool emit" << ID << "(";
-    for (auto *Arg : Args)
-      OS << Arg->getValueAsString("Name") << ", ";
+    for (size_t I = 0, N = Args.size(); I < N; ++I) {
+      const auto *Arg = Args[I];
+      bool AsRef = Arg->getValueAsBit("AsRef");
+      auto Name = Arg->getValueAsString("Name");
+
+      OS << (AsRef ? "const " : " ") << Name << " " << (AsRef ? "&" : "")
+         << ", ";
+    }
     OS << "const SourceInfo &);\n";
   });
 
@@ -275,8 +293,14 @@ void ClangOpcodesEmitter::EmitGroup(raw_ostream &OS, StringRef N,
   OS << "::" << EmitFuncName << "(";
   for (size_t I = 0, N = Types->size(); I < N; ++I)
     OS << "PrimType T" << I << ", ";
-  for (size_t I = 0, N = Args.size(); I < N; ++I)
-    OS << Args[I]->getValueAsString("Name") << " A" << I << ", ";
+  for (size_t I = 0, N = Args.size(); I < N; ++I) {
+    const auto *Arg = Args[I];
+    bool AsRef = Arg->getValueAsBit("AsRef");
+    auto Name = Arg->getValueAsString("Name");
+
+    OS << (AsRef ? "const " : " ") << Name << " " << (AsRef ? "&" : "") << "A"
+       << I << ", ";
+  }
   OS << "const SourceInfo &I) {\n";
 
   std::function<void(size_t, const Twine &)> Rec;
@@ -343,8 +367,14 @@ void ClangOpcodesEmitter::EmitEval(raw_ostream &OS, StringRef N,
               auto Args = R->getValueAsListOfDefs("Args");
 
               OS << "bool EvalEmitter::emit" << ID << "(";
-              for (size_t I = 0, N = Args.size(); I < N; ++I)
-                OS << Args[I]->getValueAsString("Name") << " A" << I << ", ";
+              for (size_t I = 0, N = Args.size(); I < N; ++I) {
+                const auto *Arg = Args[I];
+                bool AsRef = Arg->getValueAsBit("AsRef");
+                auto Name = Arg->getValueAsString("Name");
+
+                OS << (AsRef ? "const " : " ") << Name << " "
+                   << (AsRef ? "&" : "") << "A" << I << ", ";
+              }
               OS << "const SourceInfo &L) {\n";
               OS << "  if (!isActive()) return true;\n";
               OS << "  CurrentSource = L;\n";

@tbaederr
Copy link
Contributor Author

tbaederr commented Jan 28, 2024

For reference, the generated code with this patch looks like this:

#if defined(GET_EVAL_PROTO) || defined(GET_LINK_PROTO)
bool emitConstFloat(const Floating &, const SourceInfo &);
#endif
#ifdef GET_LINK_IMPL
bool ByteCodeEmitter::emitConstFloat(const Floating &A0, const SourceInfo &L) {
  return emitOp<Floating>(OP_ConstFloat, A0, L);
}
#endif
#ifdef GET_EVAL_IMPL
bool EvalEmitter::emitConstFloat(const Floating &A0, const SourceInfo &L) {
  if (!isActive()) return true;
  CurrentSource = L;
  return Const<PT_Float>(S, OpPC, A0);
}
#endif

Where all those const Floating & arguments were just Floating before:

#if defined(GET_EVAL_PROTO) || defined(GET_LINK_PROTO)
bool emitConstFloat(Floating, const SourceInfo &);
#endif
#ifdef GET_LINK_IMPL
bool ByteCodeEmitter::emitConstFloat(Floating A0, const SourceInfo &L) {
  return emitOp<Floating>(OP_ConstFloat, A0, L);
}
#endif
#ifdef GET_EVAL_IMPL
bool EvalEmitter::emitConstFloat(Floating A0, const SourceInfo &L) {
  if (!isActive()) return true;
  CurrentSource = L;
  return Const<PT_Float>(S, OpPC, A0);
}
#endif

The Float print type is backed by the Floating class, which
in turn uses APFloat, which might heap-allocate memory, so might be
expensive to copy.

Add an 'AsRef' bit to the ArgType tablegen class, which defines whether
we pass the argument around by copy or by reference.
@tbaederr
Copy link
Contributor Author

Ping

1 similar comment
@tbaederr
Copy link
Contributor Author

Ping

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM!

@tbaederr tbaederr merged commit b5437c8 into llvm:main Feb 21, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

3 participants