Skip to content

Commit 019b1f9

Browse files
committed
Handle side-effecting argument expressions
This slightly tweaks the AST formulation to wrap any potentially side-effecting initialization list expression in an OpaqueValueExpr so that during codegen we can ensure it only gets emitted once. I've made a slightly hacky but minimal change in CodeGen to then emit OpaqueValueExprs in InitLists during aggregate initialization emission. I weighed the tradeoff between an AST-level representation or this CodeGen change. If HLSL were going to retain this initialization list behavior long term, I'd probably add a new AST node to represent HLSL initialization lists and restructure how we generate the AST, but I think that would be a lot more code to maintain, and since the goal is to remove this quirk from the language I don't think it is the best solution. The change as written isolates most of the weirdness of HLSL in CGHLSLRuntime and is a massively smaller change than a new AST node for initialization lists. This will also ensure that Clang's AST-based analysis for initialization lists will continue to be accurate for HLSL without any additional updates required.
1 parent e923490 commit 019b1f9

File tree

6 files changed

+59
-1
lines changed

6 files changed

+59
-1
lines changed

clang/lib/CodeGen/CGExpr.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5551,6 +5551,12 @@ CodeGenFunction::getOrCreateOpaqueRValueMapping(const OpaqueValueExpr *e) {
55515551
return EmitAnyExpr(e->getSourceExpr());
55525552
}
55535553

5554+
bool CodeGenFunction::isOpaqueValueEmitted(const OpaqueValueExpr *E) {
5555+
if (OpaqueValueMapping::shouldBindAsLValue(E))
5556+
return OpaqueLValues.contains(E);
5557+
return OpaqueRValues.contains(E);
5558+
}
5559+
55545560
RValue CodeGenFunction::EmitRValueForField(LValue LV,
55555561
const FieldDecl *FD,
55565562
SourceLocation Loc) {

clang/lib/CodeGen/CGExprAgg.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#include "CGCXXABI.h"
14+
#include "CGHLSLRuntime.h"
1415
#include "CGObjCRuntime.h"
1516
#include "CGRecordLayout.h"
1617
#include "CodeGenFunction.h"
@@ -1776,6 +1777,16 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
17761777
}
17771778
#endif
17781779

1780+
// HLSL initialization lists in the AST are an expansion which can contain
1781+
// side-effecting expressions wrapped in opaque value expressions. To properly
1782+
// emit these we need to emit the opaque values before we emit the argument
1783+
// expressions themselves. This is a little hacky, but it prevents us needing
1784+
// to do a bigger AST-level change for a language feature that we need
1785+
// deprecate in the near future.
1786+
if (CGF.getLangOpts().HLSL && isa<InitListExpr>(ExprToVisit))
1787+
CGF.CGM.getHLSLRuntime().emitInitListOpaqueValues(
1788+
CGF, cast<InitListExpr>(ExprToVisit));
1789+
17791790
AggValueSlot Dest = EnsureSlot(ExprToVisit->getType());
17801791

17811792
LValue DestLV = CGF.MakeAddrLValue(Dest.getAddress(), ExprToVisit->getType());

clang/lib/CodeGen/CGHLSLRuntime.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414

1515
#include "CGHLSLRuntime.h"
1616
#include "CGDebugInfo.h"
17+
#include "CodeGenFunction.h"
1718
#include "CodeGenModule.h"
1819
#include "TargetInfo.h"
1920
#include "clang/AST/Decl.h"
21+
#include "clang/AST/RecursiveASTVisitor.h"
2022
#include "clang/Basic/TargetOptions.h"
2123
#include "llvm/IR/GlobalVariable.h"
2224
#include "llvm/IR/LLVMContext.h"
@@ -617,3 +619,33 @@ llvm::Instruction *CGHLSLRuntime::getConvergenceToken(BasicBlock &BB) {
617619
llvm_unreachable("Convergence token should have been emitted.");
618620
return nullptr;
619621
}
622+
623+
class OpaqueValueVisitor : public RecursiveASTVisitor<OpaqueValueVisitor> {
624+
public:
625+
llvm::SmallPtrSet<OpaqueValueExpr *, 8> OVEs;
626+
OpaqueValueVisitor() {}
627+
628+
bool VisitOpaqueValueExpr(OpaqueValueExpr *E) {
629+
OVEs.insert(E);
630+
return true;
631+
}
632+
};
633+
634+
void CGHLSLRuntime::emitInitListOpaqueValues(CodeGenFunction &CGF,
635+
InitListExpr *E) {
636+
637+
typedef CodeGenFunction::OpaqueValueMappingData OpaqueValueMappingData;
638+
OpaqueValueVisitor Visitor;
639+
Visitor.TraverseStmt(E);
640+
for (auto *OVE : Visitor.OVEs) {
641+
if (CGF.isOpaqueValueEmitted(OVE))
642+
continue;
643+
if (OpaqueValueMappingData::shouldBindAsLValue(OVE)) {
644+
LValue LV = CGF.EmitLValue(OVE->getSourceExpr());
645+
OpaqueValueMappingData::bind(CGF, OVE, LV);
646+
} else {
647+
RValue RV = CGF.EmitAnyExpr(OVE->getSourceExpr());
648+
OpaqueValueMappingData::bind(CGF, OVE, RV);
649+
}
650+
}
651+
}

clang/lib/CodeGen/CGHLSLRuntime.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class StructType;
5555
namespace clang {
5656
class VarDecl;
5757
class ParmVarDecl;
58+
class InitListExpr;
5859
class HLSLBufferDecl;
5960
class HLSLResourceBindingAttr;
6061
class Type;
@@ -65,6 +66,7 @@ class FunctionDecl;
6566
namespace CodeGen {
6667

6768
class CodeGenModule;
69+
class CodeGenFunction;
6870

6971
class CGHLSLRuntime {
7072
public:
@@ -161,6 +163,8 @@ class CGHLSLRuntime {
161163

162164
llvm::Instruction *getConvergenceToken(llvm::BasicBlock &BB);
163165

166+
void emitInitListOpaqueValues(CodeGenFunction &CGF, InitListExpr *E);
167+
164168
private:
165169
void addBufferResourceAnnotation(llvm::GlobalVariable *GV,
166170
llvm::hlsl::ResourceClass RC,

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3011,6 +3011,10 @@ class CodeGenFunction : public CodeGenTypeCache {
30113011
/// otherwise create one.
30123012
RValue getOrCreateOpaqueRValueMapping(const OpaqueValueExpr *e);
30133013

3014+
/// isOpaqueValueEmitted - Return true if the opaque value expression has
3015+
/// already been emitted.
3016+
bool isOpaqueValueEmitted(const OpaqueValueExpr *E);
3017+
30143018
/// Get the index of the current ArrayInitLoopExpr, if any.
30153019
llvm::Value *getArrayInitIndex() { return ArrayInitIndex; }
30163020

clang/lib/Sema/SemaHLSL.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3225,7 +3225,8 @@ bool SemaHLSL::TransformInitList(const InitializedEntity &Entity,
32253225
QualType Ty = E->getType();
32263226
if (auto *RTy = Ty->getAs<RecordType>())
32273227
E = new (Ctx) MaterializeTemporaryExpr(Ty, E, E->isLValue());
3228-
E = new (Ctx) OpaqueValueExpr(E->getBeginLoc(), Ty, E->getValueKind());
3228+
E = new (Ctx) OpaqueValueExpr(E->getBeginLoc(), Ty, E->getValueKind(),
3229+
E->getObjectKind(), E);
32293230
Init->setInit(I, E);
32303231
}
32313232
if (!BuildInitializerList(SemaRef, Ctx, E, ArgExprs, DestTypes))

0 commit comments

Comments
 (0)