Skip to content

Conversation

@tbaederr
Copy link
Contributor

If the getIntegerConstantExpr() calls succeed, we can as well replace the BinaryOperator children with ConstantExprs wrapping the previous LHS/RHS.

This shows small speedups: https://llvm-compile-time-tracker.com/compare.php?from=cc8c941e17558ba427de06e72c8ad96d7b17ced1&to=112af8e62e734938547d50eeb7b416c8dd666f45&stat=instructions:u

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

llvmbot commented Jul 31, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

If the getIntegerConstantExpr() calls succeed, we can as well replace the BinaryOperator children with ConstantExprs wrapping the previous LHS/RHS.

This shows small speedups: https://llvm-compile-time-tracker.com/compare.php?from=cc8c941e17558ba427de06e72c8ad96d7b17ced1&to=112af8e62e734938547d50eeb7b416c8dd666f45&stat=instructions:u


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

1 Files Affected:

  • (modified) clang/lib/Sema/SemaChecking.cpp (+17-1)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index c74b67106ad74..9b2eed3f5270f 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11374,11 +11374,27 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
         LHS->getIntegerConstantExpr(S.Context);
 
     // We don't care about expressions whose result is a constant.
-    if (RHSValue && LHSValue)
+    if (RHSValue && LHSValue) {
+      auto *LHSCE = ConstantExpr::Create(S.Context, LHS, APValue(*LHSValue));
+      auto *RHSCE = ConstantExpr::Create(S.Context, RHS, APValue(*RHSValue));
+      E->setLHS(LHSCE);
+      E->setRHS(RHSCE);
       return AnalyzeImpConvsInComparison(S, E);
+    }
 
     // We only care about expressions where just one side is literal
     if ((bool)RHSValue ^ (bool)LHSValue) {
+
+      if (LHSValue) {
+        auto *LHSCE = ConstantExpr::Create(S.Context, LHS, APValue(*LHSValue));
+        E->setLHS(LHSCE);
+        LHS = LHSCE;
+      } else if (RHSValue) {
+        auto *RHSCE = ConstantExpr::Create(S.Context, RHS, APValue(*RHSValue));
+        E->setRHS(RHSCE);
+        RHS = RHSCE;
+      }
+
       // Is the constant on the RHS or LHS?
       const bool RhsConstant = (bool)RHSValue;
       Expr *Const = RhsConstant ? RHS : LHS;

@tbaederr
Copy link
Contributor Author

Oof, looks like I mixed up the patches locally when testing and didn't see this one's breakage.

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

How do we decide where to insert ConstantExprs? Just wherever it feels convenient? Existing places where we construct a ConstantExpr are places where the language requires constant evaluation.

How does this interact with __builtin_is_constant_evaluated()?

@tbaederr tbaederr closed this Aug 10, 2025
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.

3 participants