[clang][bytecode] Use SmallVector for Function::Code

[tbaederr]

This way we can use resize_for_overwrite, which is slightly more efficient:
https://llvm-compile-time-tracker.com/compare.php?from=7bdab76350970a3ac471da6a30035dd5b7ef14f3&to=b55bd2c74f230e2150e54b0523db6a8426eab54d&stat=instructions:u

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

This way we can use resize_for_overwrite, which is slightly more efficient.

---

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

4 Files Affected:

• (modified) clang/lib/AST/ByteCode/ByteCodeEmitter.cpp (+9-9)
• (modified) clang/lib/AST/ByteCode/ByteCodeEmitter.h (+1-1)
• (modified) clang/lib/AST/ByteCode/Context.cpp (+4-4)
• (modified) clang/lib/AST/ByteCode/Function.h (+2-2)

diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp b/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
index 3288585683c10..03f47973a1138 100644
--- a/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
+++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
@@ -135,7 +135,7 @@ int32_t ByteCodeEmitter::getOffset(LabelTy Label) {
 /// Helper to write bytecode and bail out if 32-bit offsets become invalid.
 /// Pointers will be automatically marshalled as 32-bit IDs.
 template <typename T>
-static void emit(Program &P, std::vector<std::byte> &Code, const T &Val,
+static void emit(Program &P, llvm::SmallVector<std::byte> &Code, const T &Val,
                  bool &Success) {
   size_t Size;
 
@@ -153,7 +153,7 @@ static void emit(Program &P, std::vector<std::byte> &Code, const T &Val,
   size_t ValPos = align(Code.size());
   Size = align(Size);
   assert(aligned(ValPos + Size));
-  Code.resize(ValPos + Size);
+  Code.resize_for_overwrite(ValPos + Size);
 
   if constexpr (!std::is_pointer_v<T>) {
     new (Code.data() + ValPos) T(Val);
@@ -166,7 +166,7 @@ static void emit(Program &P, std::vector<std::byte> &Code, const T &Val,
 /// Emits a serializable value. These usually (potentially) contain
 /// heap-allocated memory and aren't trivially copyable.
 template <typename T>
-static void emitSerialized(std::vector<std::byte> &Code, const T &Val,
+static void emitSerialized(llvm::SmallVector<std::byte> &Code, const T &Val,
                            bool &Success) {
   size_t Size = Val.bytesToSerialize();
 
@@ -180,31 +180,31 @@ static void emitSerialized(std::vector<std::byte> &Code, const T &Val,
   size_t ValPos = Code.size();
   Size = align(Size);
   assert(aligned(ValPos + Size));
-  Code.resize(ValPos + Size);
+  Code.resize_for_overwrite(ValPos + Size);
 
   Val.serialize(Code.data() + ValPos);
 }
 
 template <>
-void emit(Program &P, std::vector<std::byte> &Code, const Floating &Val,
+void emit(Program &P, llvm::SmallVector<std::byte> &Code, const Floating &Val,
           bool &Success) {
   emitSerialized(Code, Val, Success);
 }
 
 template <>
-void emit(Program &P, std::vector<std::byte> &Code,
+void emit(Program &P, llvm::SmallVector<std::byte> &Code,
           const IntegralAP<false> &Val, bool &Success) {
   emitSerialized(Code, Val, Success);
 }
 
 template <>
-void emit(Program &P, std::vector<std::byte> &Code, const IntegralAP<true> &Val,
-          bool &Success) {
+void emit(Program &P, llvm::SmallVector<std::byte> &Code,
+          const IntegralAP<true> &Val, bool &Success) {
   emitSerialized(Code, Val, Success);
 }
 
 template <>
-void emit(Program &P, std::vector<std::byte> &Code, const FixedPoint &Val,
+void emit(Program &P, llvm::SmallVector<std::byte> &Code, const FixedPoint &Val,
           bool &Success) {
   emitSerialized(Code, Val, Success);
 }
diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.h b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
index 9e9dd5e87cd7a..8a02911189748 100644
--- a/clang/lib/AST/ByteCode/ByteCodeEmitter.h
+++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
@@ -88,7 +88,7 @@ class ByteCodeEmitter {
   /// Location of label relocations.
   llvm::DenseMap<LabelTy, llvm::SmallVector<unsigned, 5>> LabelRelocs;
   /// Program code.
-  std::vector<std::byte> Code;
+  llvm::SmallVector<std::byte> Code;
   /// Opcode to expression mapping.
   SourceMap SrcMap;
 
diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp
index aaeb52e0fa449..bc76dde570bac 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -44,12 +44,12 @@ bool Context::isPotentialConstantExpr(State &Parent, const FunctionDecl *FD) {
   Compiler<ByteCodeEmitter>(*this, *P).compileFunc(
       FD, const_cast<Function *>(Func));
 
-  ++EvalID;
-  // And run it.
-  if (!Run(Parent, Func))
+  if (!Func->isValid())
     return false;
 
-  return Func->isValid();
+  ++EvalID;
+  // And run it.
+  return Run(Parent, Func);
 }
 
 void Context::isPotentialConstantExprUnevaluated(State &Parent, const Expr *E,
diff --git a/clang/lib/AST/ByteCode/Function.h b/clang/lib/AST/ByteCode/Function.h
index de88f3ded34dc..b5ea1b00fce10 100644
--- a/clang/lib/AST/ByteCode/Function.h
+++ b/clang/lib/AST/ByteCode/Function.h
@@ -236,7 +236,7 @@ class Function final {
            bool HasRVO, bool IsLambdaStaticInvoker);
 
   /// Sets the code of a function.
-  void setCode(unsigned NewFrameSize, std::vector<std::byte> &&NewCode,
+  void setCode(unsigned NewFrameSize, llvm::SmallVector<std::byte> &&NewCode,
                SourceMap &&NewSrcMap, llvm::SmallVector<Scope, 2> &&NewScopes,
                bool NewHasBody) {
     FrameSize = NewFrameSize;
@@ -266,7 +266,7 @@ class Function final {
   /// Size of the argument stack.
   unsigned ArgSize;
   /// Program code.
-  std::vector<std::byte> Code;
+  llvm::SmallVector<std::byte> Code;
   /// Opcode-to-expression mapping.
   SourceMap SrcMap;
   /// List of block descriptors.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[shafik]

I wonder how useful this inline comments are, I feel like that might be better and added to the comment on the declaration of isPotentialConstantExpr something like

"Check if the function is a potential constant expression by compiling the function and running it."

[tbaederr]

I don't really care either way but this comment isn't new from this patch, just moved down a bit.

[shafik]

This is a drive by nit but since you in this area already it feels like a worthwhile fix.