[clang] 9022f40 - [clang][Interp] Only evaluate the source array initialization of an `ArrayInitLoopExpr` once (#68039)

[via cfe-commits]

Author: isuckatcs
Date: 2023-10-23T17:52:27+02:00
New Revision: 9022f402120b4e55ec1201c16f8f6d2388498e1d

URL: https://github.com/llvm/llvm-project/commit/9022f402120b4e55ec1201c16f8f6d2388498e1d
DIFF: https://github.com/llvm/llvm-project/commit/9022f402120b4e55ec1201c16f8f6d2388498e1d.diff

LOG: [clang][Interp] Only evaluate the source array initialization of an `ArrayInitLoopExpr` once (#68039)

This patch implements an `OpaqueValueExpr` caching functionality in `Interp`
by storing the result of the expression in a local variable.

Added: 
    

Modified: 
    clang/lib/AST/Interp/ByteCodeExprGen.cpp
    clang/test/AST/Interp/arrays.cpp
    clang/test/AST/Interp/cxx20.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index ed971fe0f650f22..d3e0d1112935a98 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -817,8 +817,8 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
   assert(Initializing);
   assert(!DiscardResult);
   // TODO: This compiles to quite a lot of bytecode if the array is larger.
-  //   Investigate compiling this to a loop, or at least try to use
-  //   the AILE's Common expr.
+  //   Investigate compiling this to a loop.
+
   const Expr *SubExpr = E->getSubExpr();
   size_t Size = E->getArraySize().getZExtValue();
   std::optional<PrimType> ElemT = classify(SubExpr->getType());
@@ -853,7 +853,33 @@ template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
   if (Initializing)
     return this->visitInitializer(E->getSourceExpr());
-  return this->visit(E->getSourceExpr());
+
+  PrimType SubExprT = classify(E->getSourceExpr()).value_or(PT_Ptr);
+  if (auto It = OpaqueExprs.find(E); It != OpaqueExprs.end())
+    return this->emitGetLocal(SubExprT, It->getSecond(), E);
+
+  if (!this->visit(E->getSourceExpr()))
+    return false;
+
+  // At this point we either have the evaluated source expression or a pointer
+  // to an object on the stack. We want to create a local variable that stores
+  // this value.
+  std::optional<unsigned> LocalIndex =
+      allocateLocalPrimitive(E, SubExprT, /*IsConst=*/true);
+  if (!LocalIndex)
+    return false;
+  if (!this->emitSetLocal(SubExprT, *LocalIndex, E))
+    return false;
+
+  // Here the local variable is created but the value is removed from the stack,
+  // so we put it back, because the caller might need it.
+  if (!this->emitGetLocal(SubExprT, *LocalIndex, E))
+    return false;
+
+  // FIXME: Ideally the cached value should be cleaned up later.
+  OpaqueExprs.insert({E, *LocalIndex});
+
+  return true;
 }
 
 template <class Emitter>

diff  --git a/clang/test/AST/Interp/arrays.cpp b/clang/test/AST/Interp/arrays.cpp
index 18c4ae4354f54a0..ba29632e97c30a6 100644
--- a/clang/test/AST/Interp/arrays.cpp
+++ b/clang/test/AST/Interp/arrays.cpp
@@ -372,9 +372,6 @@ namespace ZeroInit {
 }
 
 namespace ArrayInitLoop {
-  /// FIXME: The ArrayInitLoop for the decomposition initializer in g() has
-  /// f(n) as its CommonExpr. We need to evaluate that exactly once and not
-  /// N times as we do right now.
   struct X {
       int arr[3];
   };
@@ -386,8 +383,7 @@ namespace ArrayInitLoop {
       auto [a, b, c] = f(n).arr;
       return a + b + c;
   }
-  static_assert(g() == 6); // expected-error {{failed}} \
-                           // expected-note {{15 == 6}}
+  static_assert(g() == 6, "");
 }
 
 namespace StringZeroFill {

diff  --git a/clang/test/AST/Interp/cxx20.cpp b/clang/test/AST/Interp/cxx20.cpp
index 197090b0a37d9df..553bc6eb4d5244f 100644
--- a/clang/test/AST/Interp/cxx20.cpp
+++ b/clang/test/AST/Interp/cxx20.cpp
@@ -733,3 +733,15 @@ namespace ConstexprArrayInitLoopExprDestructors
       return f();
   }
 }
+
+namespace NonPrimitiveOpaqueValue
+{
+  struct X {
+    int x;
+    constexpr operator bool() const { return x != 0; }
+  };
+
+  constexpr int ternary() { return X(0) ?: X(0); }
+
+  static_assert(!ternary(), "");
+}