Skip to content

[clang][OpenCL] Only evaluate initializer once to check for zero init #141474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 27, 2025

Conversation

tbaederr
Copy link
Contributor

Both Expr::isIntegerConstantExpr() and Epxr::EvaluateKnownConstInt() evaluate the expression. Just do it once and check the integer result.

Both Expr::isIntegerConstantExpr() and Epxr::EvaluateKnownConstInt()
evaluate the expression. Just do it once and check the integer result.
@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

Both Expr::isIntegerConstantExpr() and Epxr::EvaluateKnownConstInt() evaluate the expression. Just do it once and check the integer result.


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

1 Files Affected:

  • (modified) clang/lib/Sema/SemaInit.cpp (+5-5)
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 2f682dbc1f000..776cb022e6925 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -6410,9 +6410,9 @@ static bool TryOCLSamplerInitialization(Sema &S,
   return true;
 }
 
-static bool IsZeroInitializer(Expr *Initializer, Sema &S) {
-  return Initializer->isIntegerConstantExpr(S.getASTContext()) &&
-    (Initializer->EvaluateKnownConstInt(S.getASTContext()) == 0);
+static bool IsZeroInitializer(const Expr *Init, ASTContext &Ctx) {
+  std::optional<llvm::APSInt> Value = Init->getIntegerConstantExpr(Ctx);
+  return Value && Value->isZero();
 }
 
 static bool TryOCLZeroOpaqueTypeInitialization(Sema &S,
@@ -6431,7 +6431,7 @@ static bool TryOCLZeroOpaqueTypeInitialization(Sema &S,
   // event should be zero.
   //
   if (DestType->isEventT() || DestType->isQueueT()) {
-    if (!IsZeroInitializer(Initializer, S))
+    if (!IsZeroInitializer(Initializer, S.getASTContext()))
       return false;
 
     Sequence.AddOCLZeroOpaqueTypeStep(DestType);
@@ -6447,7 +6447,7 @@ static bool TryOCLZeroOpaqueTypeInitialization(Sema &S,
     if (DestType->isOCLIntelSubgroupAVCMcePayloadType() ||
         DestType->isOCLIntelSubgroupAVCMceResultType())
       return false;
-    if (!IsZeroInitializer(Initializer, S))
+    if (!IsZeroInitializer(Initializer, S.getASTContext()))
       return false;
 
     Sequence.AddOCLZeroOpaqueTypeStep(DestType);

@tbaederr tbaederr requested a review from asavonic May 26, 2025 10:40
Copy link
Collaborator

@asavonic asavonic left a comment

Choose a reason for hiding this comment

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

Good catch, LGTM.

@tbaederr tbaederr merged commit 17ef0fe into llvm:main May 27, 2025
14 checks passed
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…llvm#141474)

Both Expr::isIntegerConstantExpr() and Expr::EvaluateKnownConstInt()
evaluate the expression. Just do it once and check the integer result.
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