[clang] [clang] Optimize EmbedExpr child iterator callback (PR #155803)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

<details>
<summary>Changes</summary>

Instead of querying the bitwidth and signeness of the integer literal for every iteration, get the bitwidth directly from the `APIntStorage` and assume the signeness to be `true` since we set the type of the `EmbedExpr` to `Ctx.IntTy` and the type of the integer literal to that of the `EmbedExpr`, so it should always be signed, as long as `ASTContext::IntTy` is signed.

Before:
```
$ hyperfine -r 50 -w 3 'bin/clang -c ../../benchmarks/embed.cpp -std=c++20 -fconstexpr-steps=1000000000'
Benchmark 1: bin/clang -c ../../benchmarks/embed.cpp -std=c++20 -fconstexpr-steps=1000000000
  Time (mean ± σ):      1.796 s ±  0.090 s    [User: 0.961 s, System: 0.834 s]
  Range (min … max):    1.640 s …  2.150 s    50 runs
```
After:
```
$ hyperfine -r 50 -w 3 'bin/clang -c ../../benchmarks/embed.cpp -std=c++20 -fconstexpr-steps=1000000000'
Benchmark 1: bin/clang -c ../../benchmarks/embed.cpp -std=c++20 -fconstexpr-steps=1000000000
  Time (mean ± σ):      1.700 s ±  0.050 s    [User: 0.909 s, System: 0.789 s]
  Range (min … max):    1.637 s …  1.880 s    50 runs
```

That is roughly .1s less, or whatever, 5% or something.

The benchmark is simply:
```c++
constexpr char str[] = {
#embed "sqlite3.c" suffix(,0)
};

constexpr char str2[] = {
#embed "sqlite3.c" suffix(,0)
};

constexpr char str3[] = {
#embed "sqlite3.c" suffix(,0)
};
```
where `sqlite3.c` contains the sqlite3 amalgamation (roughly 9 million characters).



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


4 Files Affected:

- (modified) clang/include/clang/AST/APNumericStorage.h (+2-1) 
- (modified) clang/include/clang/AST/Expr.h (+2-2) 
- (modified) clang/lib/AST/Expr.cpp (+1) 
- (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+1-1) 


``````````diff
diff --git a/clang/include/clang/AST/APNumericStorage.h b/clang/include/clang/AST/APNumericStorage.h
index 95eddbcd86e83..e1948a552bf7e 100644
--- a/clang/include/clang/AST/APNumericStorage.h
+++ b/clang/include/clang/AST/APNumericStorage.h
@@ -28,7 +28,6 @@ class APNumericStorage {
     uint64_t VAL;   ///< Used to store the <= 64 bits integer value.
     uint64_t *pVal; ///< Used to store the >64 bits integer value.
   };
-  unsigned BitWidth;
 
   bool hasAllocation() const { return llvm::APInt::getNumWords(BitWidth) > 1; }
 
@@ -36,6 +35,7 @@ class APNumericStorage {
   void operator=(const APNumericStorage &) = delete;
 
 protected:
+  unsigned BitWidth;
   APNumericStorage() : VAL(0), BitWidth(0) {}
 
   llvm::APInt getIntValue() const {
@@ -51,6 +51,7 @@ class APNumericStorage {
 class APIntStorage : private APNumericStorage {
 public:
   llvm::APInt getValue() const { return getIntValue(); }
+  unsigned getBitWidth() const { return BitWidth; }
   void setValue(const ASTContext &C, const llvm::APInt &Val) {
     setIntValue(C, Val);
   }
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 574ee54077ce7..23a0996f02725 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -5113,9 +5113,9 @@ class EmbedExpr final : public Expr {
              "trying to dereference an invalid iterator");
       IntegerLiteral *N = EExpr->FakeChildNode;
       N->setValue(*EExpr->Ctx,
-                  llvm::APInt(N->getValue().getBitWidth(),
+                  llvm::APInt(N->getBitWidth(),
                               EExpr->Data->BinaryData->getCodeUnit(CurOffset),
-                              N->getType()->isSignedIntegerType()));
+                              /*Signed=*/true));
       // We want to return a reference to the fake child node in the
       // EmbedExpr, not the local variable N.
       return const_cast<typename BaseTy::reference>(EExpr->FakeChildNode);
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 512f152059a01..cdff160067fed 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -2395,6 +2395,7 @@ EmbedExpr::EmbedExpr(const ASTContext &Ctx, SourceLocation Loc,
   setDependence(ExprDependence::None);
   FakeChildNode = IntegerLiteral::Create(
       Ctx, llvm::APInt::getZero(Ctx.getTypeSize(getType())), getType(), Loc);
+  assert(getType()->isSignedIntegerType() && "IntTy should be signed");
 }
 
 InitListExpr::InitListExpr(const ASTContext &C, SourceLocation lbraceloc,
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index be9bad9e96cc1..301ed9b23c206 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -730,7 +730,7 @@ void ASTStmtWriter::VisitIntegerLiteral(IntegerLiteral *E) {
   Record.AddSourceLocation(E->getLocation());
   Record.AddAPInt(E->getValue());
 
-  if (E->getValue().getBitWidth() == 32) {
+  if (E->getBitWidth() == 32) {
     AbbrevToUse = Writer.getIntegerLiteralAbbrev();
   }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/155803