[clang][bytecode] Surround bcp condition with Start/EndSpeculation

[tbaederr]

This is similar to what the current interpreter is doing - the FoldConstant RAII object surrounds the entire HandleConditionalOperator call, which means the condition and both TrueExpr or FalseExpr.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

This is similar to what the current interpreter is doing - the FoldConstant RAII object surrounds the entire HandleConditionalOperator call, which means the condition and both TrueExpr or FalseExpr.

---

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

2 Files Affected:

• (modified) clang/lib/AST/ByteCode/Compiler.cpp (+13-6)
• (modified) clang/test/AST/ByteCode/builtin-constant-p.cpp (+2-5)

diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 9a52dd4105437..13b8a3b47add6 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -2309,29 +2309,36 @@ bool Compiler<Emitter>::VisitAbstractConditionalOperator(
     return visitChildExpr(FalseExpr);
   }
 
+  bool IsBcpCall = false;
+  if (const auto *CE = dyn_cast<CallExpr>(Condition->IgnoreParenCasts());
+      CE && CE->getBuiltinCallee() == Builtin::BI__builtin_constant_p) {
+    IsBcpCall = true;
+  }
+
   LabelTy LabelEnd = this->getLabel();   // Label after the operator.
   LabelTy LabelFalse = this->getLabel(); // Label for the false expr.
 
+  if (IsBcpCall) {
+    if (!this->emitStartSpeculation(E))
+      return false;
+  }
+
   if (!this->visitBool(Condition))
     return false;
-
   if (!this->jumpFalse(LabelFalse))
     return false;
-
   if (!visitChildExpr(TrueExpr))
     return false;
-
   if (!this->jump(LabelEnd))
     return false;
-
   this->emitLabel(LabelFalse);
-
   if (!visitChildExpr(FalseExpr))
     return false;
-
   this->fallthrough(LabelEnd);
   this->emitLabel(LabelEnd);
 
+  if (IsBcpCall)
+    return this->emitEndSpeculation(E);
   return true;
 }
 
diff --git a/clang/test/AST/ByteCode/builtin-constant-p.cpp b/clang/test/AST/ByteCode/builtin-constant-p.cpp
index b309fa8296889..ed9e606ed16aa 100644
--- a/clang/test/AST/ByteCode/builtin-constant-p.cpp
+++ b/clang/test/AST/ByteCode/builtin-constant-p.cpp
@@ -59,13 +59,10 @@ template<typename T> constexpr bool bcp(T t) {
 }
 
 constexpr intptr_t ptr_to_int(const void *p) {
-  return __builtin_constant_p(1) ? (intptr_t)p : (intptr_t)p; // expected-note {{cast that performs the conversions of a reinterpret_cast}}
+  return __builtin_constant_p(1) ? (intptr_t)p : (intptr_t)p;
 }
 
-/// This is from test/SemaCXX/builtin-constant-p.cpp, but it makes no sense.
-/// ptr_to_int is called before bcp(), so it fails. GCC does not accept this either.
-static_assert(bcp(ptr_to_int("foo"))); // expected-error {{not an integral constant expression}} \
-                                       // expected-note {{in call to}}
+static_assert(bcp(ptr_to_int("foo")));
 
 constexpr bool AndFold(const int &a, const int &b) {
   return __builtin_constant_p(a && b);

[llvm-ci]

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-sie-win running on sie-win-worker while building clang at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/13232

Here is the relevant piece of the build log for the reference

Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/6/44' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:Z:\b\llvm-clang-x86_64-sie-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-11736-6-44.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=44 GTEST_SHARD_INDEX=6 Z:\b\llvm-clang-x86_64-sie-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
Z:\b\llvm-clang-x86_64-sie-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=Caching.NoCommit
--
Z:\b\llvm-clang-x86_64-sie-win\llvm-project\llvm\unittests\Support\Caching.cpp(142): error: Value of: AddStream
  Actual: false
Expected: true


Z:\b\llvm-clang-x86_64-sie-win\llvm-project\llvm\unittests\Support\Caching.cpp:142
Value of: AddStream
  Actual: false
Expected: true



********************