Skip to content

Commit 9022f40

Browse files
authored
[clang][Interp] Only evaluate the source array initialization of an ArrayInitLoopExpr once (llvm#68039)
This patch implements an `OpaqueValueExpr` caching functionality in `Interp` by storing the result of the expression in a local variable.
1 parent 4b15c0e commit 9022f40

File tree

3 files changed

+42
-8
lines changed

3 files changed

+42
-8
lines changed

clang/lib/AST/Interp/ByteCodeExprGen.cpp

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -817,8 +817,8 @@ bool ByteCodeExprGen<Emitter>::VisitArrayInitLoopExpr(
817817
assert(Initializing);
818818
assert(!DiscardResult);
819819
// TODO: This compiles to quite a lot of bytecode if the array is larger.
820-
// Investigate compiling this to a loop, or at least try to use
821-
// the AILE's Common expr.
820+
// Investigate compiling this to a loop.
821+
822822
const Expr *SubExpr = E->getSubExpr();
823823
size_t Size = E->getArraySize().getZExtValue();
824824
std::optional<PrimType> ElemT = classify(SubExpr->getType());
@@ -853,7 +853,33 @@ template <class Emitter>
853853
bool ByteCodeExprGen<Emitter>::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
854854
if (Initializing)
855855
return this->visitInitializer(E->getSourceExpr());
856-
return this->visit(E->getSourceExpr());
856+
857+
PrimType SubExprT = classify(E->getSourceExpr()).value_or(PT_Ptr);
858+
if (auto It = OpaqueExprs.find(E); It != OpaqueExprs.end())
859+
return this->emitGetLocal(SubExprT, It->getSecond(), E);
860+
861+
if (!this->visit(E->getSourceExpr()))
862+
return false;
863+
864+
// At this point we either have the evaluated source expression or a pointer
865+
// to an object on the stack. We want to create a local variable that stores
866+
// this value.
867+
std::optional<unsigned> LocalIndex =
868+
allocateLocalPrimitive(E, SubExprT, /*IsConst=*/true);
869+
if (!LocalIndex)
870+
return false;
871+
if (!this->emitSetLocal(SubExprT, *LocalIndex, E))
872+
return false;
873+
874+
// Here the local variable is created but the value is removed from the stack,
875+
// so we put it back, because the caller might need it.
876+
if (!this->emitGetLocal(SubExprT, *LocalIndex, E))
877+
return false;
878+
879+
// FIXME: Ideally the cached value should be cleaned up later.
880+
OpaqueExprs.insert({E, *LocalIndex});
881+
882+
return true;
857883
}
858884

859885
template <class Emitter>

clang/test/AST/Interp/arrays.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -372,9 +372,6 @@ namespace ZeroInit {
372372
}
373373

374374
namespace ArrayInitLoop {
375-
/// FIXME: The ArrayInitLoop for the decomposition initializer in g() has
376-
/// f(n) as its CommonExpr. We need to evaluate that exactly once and not
377-
/// N times as we do right now.
378375
struct X {
379376
int arr[3];
380377
};
@@ -386,8 +383,7 @@ namespace ArrayInitLoop {
386383
auto [a, b, c] = f(n).arr;
387384
return a + b + c;
388385
}
389-
static_assert(g() == 6); // expected-error {{failed}} \
390-
// expected-note {{15 == 6}}
386+
static_assert(g() == 6, "");
391387
}
392388

393389
namespace StringZeroFill {

clang/test/AST/Interp/cxx20.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,3 +733,15 @@ namespace ConstexprArrayInitLoopExprDestructors
733733
return f();
734734
}
735735
}
736+
737+
namespace NonPrimitiveOpaqueValue
738+
{
739+
struct X {
740+
int x;
741+
constexpr operator bool() const { return x != 0; }
742+
};
743+
744+
constexpr int ternary() { return X(0) ?: X(0); }
745+
746+
static_assert(!ternary(), "");
747+
}

0 commit comments

Comments
 (0)