Skip to content

Conversation

@tbaederr
Copy link
Contributor

@tbaederr tbaederr commented May 26, 2025

... Iteration.

The isIgnored() check is more expensive than the earlier checks, so move it last.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 26, 2025
@llvmbot
Copy link
Member

llvmbot commented May 26, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

... Iteration.


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

1 Files Affected:

  • (modified) clang/lib/Sema/SemaStmt.cpp (+4-4)
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index ed070649dc1f9..6d6da6e7fdd8c 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -2207,16 +2207,16 @@ namespace {
     // Return when there is nothing to check.
     if (!Body || !Third) return;
 
-    if (S.Diags.isIgnored(diag::warn_redundant_loop_iteration,
-                          Third->getBeginLoc()))
-      return;
-
     // Get the last statement from the loop body.
     CompoundStmt *CS = dyn_cast<CompoundStmt>(Body);
     if (!CS || CS->body_empty()) return;
     Stmt *LastStmt = CS->body_back();
     if (!LastStmt) return;
 
+    if (S.Diags.isIgnored(diag::warn_redundant_loop_iteration,
+                          Third->getBeginLoc()))
+      return;
+
     bool LoopIncrement, LastIncrement;
     DeclRefExpr *LoopDRE, *LastDRE;
 

@tbaederr tbaederr requested a review from cor3ntin May 26, 2025 10:25
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

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

Can you elaborate in your summary.

I am guessing because the other checks are a lot cheaper and this overall improves performance?

@tbaederr
Copy link
Contributor Author

Can you elaborate in your summary.

I am guessing because the other checks are a lot cheaper and this overall improves performance?

Yep. Updated the description.

Copy link
Collaborator

@shafik shafik 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 ccb6b0d into llvm:main May 30, 2025
14 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.

4 participants