llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Kamil Jakubus (jkbz64)
<details>
<summary>Changes</summary>
## Problem
Consider a sparse lookup-table row:
```c
static const unsigned short table[8] = {
[2] = 7,
[7] = 42,
};
```
Clang's semantic initializer list contains eight entries. The six omitted
elements refer to a shared `ImplicitValueInitExpr` representing zero
initialization. Several AST and Sema traversals revisit these entries
redundantly, adding work for each omitted element across large tables.
## Motivation
See #<!-- -->227592 but this improves compile speed only.
## C Changes
- `Expr::isConstantInitializer` accepts `ImplicitValueInitExpr` directly
instead of calling the full checker for each occurrence.
- Conversion and integer-overflow diagnostic walks, and the
unsequenced-operation visitor, skip implicit value-initialization nodes before
queuing or visiting them.
- `Expr::HasSideEffects` checks the array filler once, then skips entries
pointing to that same expression. Explicit initializers still receive their
normal checks.
These loops still inspect every initializer slot. The savings come from fewer
expression checks, worklist entries and visitor calls; the semantic initializer
representation stays the same.
## C++ Changes
The shared constant-initializer, side-effect, conversion and overflow changes
also apply to C++. Nontrivial fillers still receive their normal side-effect
check. The sequencing shortcut applies to C++98/03; C++11 and later retain
`SequenceExpressionsInOrder`.
## Additional changes
Refactored a few for-loops with `llvm::{all_of,any_of,copy_if}` where it
improved readability, let me know if that's not desirable, there was no
performance difference and it looks better (?).
Added a few test cases that might not be perfect but were uncovered, doesn't
hurt I guess?
## Measured impact
Unpatched Clang `082fe79e68b81352290afcd8661874bed7e61074` versus this patch.
Both are Release builds with assertions disabled, running on AArch64 (Apple M1
Pro), and compile identical numeric `parser.c` inputs generated by upstream
Tree-sitter 0.27.0.
Times are medians of five alternating runs per compiler.
The measurements below are independent of the allocation changes in #<!--
-->227592.
| Parser | Compile before | Compile after | Time reduction |
|---|---:|---:|---:|
| [ABL](https://github.com/usagi-coffee/tree-sitter-abl) | 8.374 s | 7.787 s |
7.0% |
|
[Swift](https://github.com/alex-pinkus/tree-sitter-swift/tree/35245fbfee2fccf16273c6f4299438fb76875970)
| 1.126 s | 1.094 s | 2.8% |
|
[F#](https://github.com/ionide/tree-sitter-fsharp/tree/aefd0c8741bdf3aeb827a228aa4a996a6536697e)
| 2.520 s | 2.422 s | 3.9% |
|
[Scala](https://github.com/tree-sitter/tree-sitter-scala/tree/db390f312a54b04b13790e1767bfac32665c17ac)
| 1.321 s | 1.282 s | 2.9% |
|
[Kotlin](https://github.com/tree-sitter-grammars/tree-sitter-kotlin/tree/3dea6dfa9c0129deb7c4315afbda806c85c41667)
| 1.056 s | 1.027 s | 2.8% |
|
[Julia](https://github.com/tree-sitter/tree-sitter-julia/tree/e0f9dcd180fdcfcfa8d79a3531e11d99e79321d3)
| 0.954 s | 0.927 s | 2.9% |
|
[C#](https://github.com/tree-sitter/tree-sitter-c-sharp/tree/8c0abe0b84a3681d3e6852ce2e2a1eecb4f731c5)
| 1.343 s | 1.289 s | 4.0% |
|
[C++](https://github.com/tree-sitter/tree-sitter-cpp/tree/c009222808634c1014f82438d4883753516a2c24)
| 1.202 s | 1.163 s | 3.2% |
|
[Vim](https://github.com/tree-sitter-grammars/tree-sitter-vim/tree/039c8d0aa1deae00ddeb0374dd70bcc0ec56938d)
| 0.758 s | 0.741 s | 2.2% |
```sh
clang --gcc-triple=aarch64-redhat-linux -std=c11 -O0 \
-fPIC -ffunction-sections -fdata-sections \
-I src -c src/parser.c -o parser.o
```
---
Full diff: https://github.com/llvm/llvm-project/pull/227671.diff
3 Files Affected:
- (modified) clang/lib/AST/Expr.cpp (+18-12)
- (modified) clang/lib/Sema/SemaChecking.cpp (+15-6)
- (modified) clang/test/Sema/designated-initializers.c (+20)
``````````diff
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 728be799c50a3..21a7794c7ec35 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -34,6 +34,7 @@
#include "clang/Lex/Lexer.h"
#include "clang/Lex/LiteralSupport.h"
#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/raw_ostream.h"
@@ -3460,12 +3461,11 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool
IsForRef,
return ILE->getInit(0)->isConstantInitializer(Ctx, false, Culprit);
if (ILE->getType()->isArrayType()) {
- unsigned numInits = ILE->getNumInits();
- for (unsigned i = 0; i < numInits; i++) {
- if (!ILE->getInit(i)->isConstantInitializer(Ctx, false, Culprit))
- return false;
- }
- return true;
+ // Implicit zero initializers are constant without further evaluation.
+ return llvm::all_of(ILE->inits(), [&](const Expr *Init) {
+ return isa<ImplicitValueInitExpr>(Init) ||
+ Init->isConstantInitializer(Ctx, false, Culprit);
+ });
}
if (ILE->getType()->isRecordType()) {
@@ -3885,12 +3885,18 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
return true;
break;
- case InitListExprClass:
- // FIXME: The children for an InitListExpr doesn't include the array
filler.
- if (const Expr *E = cast<InitListExpr>(this)->getArrayFiller())
- if (E->HasSideEffects(Ctx, IncludePossibleEffects))
- return true;
- break;
+ case InitListExprClass: {
+ const auto *ILE = cast<InitListExpr>(this);
+ // The filler also occupies every hole in a sparse initializer. Checking
+ // its side effects once suffices, even when it occurs many times.
+ const Expr *Filler = ILE->getArrayFiller();
+ if (Filler && Filler->HasSideEffects(Ctx, IncludePossibleEffects))
+ return true;
+ return llvm::any_of(ILE->inits(), [&](const Expr *Init) {
+ return Init && Init != Filler &&
+ Init->HasSideEffects(Ctx, IncludePossibleEffects);
+ });
+ }
case GenericSelectionExprClass:
return cast<GenericSelectionExpr>(this)->getResultExpr()->HasSideEffects(
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index c0cfc51f5b68f..825c6a8877a77 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -113,6 +113,7 @@
#include <cstddef>
#include <cstdint>
#include <functional>
+#include <iterator>
#include <limits>
#include <optional>
#include <string>
@@ -14295,7 +14296,9 @@ static void AnalyzeImplicitConversions(
bool IsLogicalAndOperator = BO && BO->getOpcode() == BO_LAnd;
for (Stmt *SubStmt : E->children()) {
Expr *ChildExpr = dyn_cast_or_null<Expr>(SubStmt);
- if (!ChildExpr)
+ // Value initialization has no implicit conversions to diagnose. Sparse
+ // array initializers can contain many references to the same zero filler.
+ if (!ChildExpr || isa<ImplicitValueInitExpr>(ChildExpr))
continue;
if (auto *CSE = dyn_cast<CoroutineSuspendExpr>(E))
@@ -14668,9 +14671,11 @@ void Sema::CheckForIntOverflow (const Expr *E) {
continue;
}
- if (const auto *InitList = dyn_cast<InitListExpr>(OriginalE))
- Exprs.append(InitList->inits().begin(), InitList->inits().end());
- else if (isa<ObjCBoxedExpr>(OriginalE))
+ if (const auto *InitList = dyn_cast<InitListExpr>(OriginalE)) {
+ llvm::copy_if(
+ InitList->inits(), std::back_inserter(Exprs),
+ [](const Expr *Init) { return !isa<ImplicitValueInitExpr>(Init); });
+ } else if (isa<ObjCBoxedExpr>(OriginalE))
E->EvaluateForOverflow(Context);
else if (const auto *Call = dyn_cast<CallExpr>(E))
Exprs.append(Call->arg_begin(), Call->arg_end());
@@ -15528,8 +15533,12 @@ class SequenceChecker : public
ConstEvaluatedExprVisitor<SequenceChecker> {
}
void VisitInitListExpr(const InitListExpr *ILE) {
- if (!SemaRef.getLangOpts().CPlusPlus11)
- return VisitExpr(ILE);
+ if (!SemaRef.getLangOpts().CPlusPlus11) {
+ for (const Expr *Init : ILE->inits())
+ if (Init && !isa<ImplicitValueInitExpr>(Init))
+ Visit(Init);
+ return;
+ }
// In C++11, list initializations are sequenced.
SequenceExpressionsInOrder(ILE->inits());
diff --git a/clang/test/Sema/designated-initializers.c
b/clang/test/Sema/designated-initializers.c
index 11dc3a2308dee..7b0c3387de1b8 100644
--- a/clang/test/Sema/designated-initializers.c
+++ b/clang/test/Sema/designated-initializers.c
@@ -375,3 +375,23 @@ void gh154046(void) {
[1] = "" // expected-error {{incompatible pointer to integer conversion
initializing 'const char' with an expression of type 'char[1]'}}
}[1];
}
+
+// Implicit zero elements must not hide diagnostics for explicit initializers.
+void sparse_diagnostics(int i) {
+ unsigned char conversion[128] = {
+ [127] = 256 // expected-warning {{implicit conversion from 'int' to
'unsigned char' changes value from 256 to 0}}
+ };
+ int overflow[128] = {
+ [127] = 2147483647 + 1 // expected-warning {{overflow in expression;
result is -2'147'483'648 with type 'int'}}
+ };
+ int unsequenced[128] = {
+ [127] = i++ + i // expected-warning {{unsequenced modification and access
to 'i'}}
+ };
+ static int nonconstant[128] = {
+ [127] = i // expected-error {{initializer element is not a compile-time
constant}}
+ };
+ struct { int row[128]; } overridden = {
+ .row = {[127] = i++}, // expected-note {{previous initialization with side
effects is here (side effects will not occur at run time)}}
+ .row = {[1] = 1} // expected-warning {{initializer overrides prior
initialization of this subobject}}
+ };
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/227671
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
