Skip to content

Commit dab8c88

Browse files
authored
[OpenACC] Implement 'firstprivate' clause copy lowering (#154150)
This patch is the last of the 'firstprivate' clause lowering patches. It takes the already generated 'copy' init from Sema and uses it to generate the IR for the copy section of the recipe. However, one thing that this patch had to do, was come up with a way to hijack the decl registration in CIRGenFunction. Because these decls are being created in a 'different' place, we need to remove the things we've added. We could alternatively generate these 'differently', but it seems worth a little extra effort here to avoid having to re-implement variable initialization.
1 parent 01f2d70 commit dab8c88

File tree

6 files changed

+522
-37
lines changed

6 files changed

+522
-37
lines changed

clang/lib/CIR/CodeGen/CIRGenDecl.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -154,15 +154,19 @@ void CIRGenFunction::emitAutoVarInit(
154154
initializeWhatIsTechnicallyUninitialized(addr);
155155
LValue lv = makeAddrLValue(addr, type, AlignmentSource::Decl);
156156
emitExprAsInit(init, &d, lv);
157-
// In case lv has uses it means we indeed initialized something
158-
// out of it while trying to build the expression, mark it as such.
159-
mlir::Value val = lv.getAddress().getPointer();
160-
assert(val && "Should have an address");
161-
auto allocaOp = val.getDefiningOp<cir::AllocaOp>();
162-
assert(allocaOp && "Address should come straight out of the alloca");
163-
164-
if (!allocaOp.use_empty())
165-
allocaOp.setInitAttr(mlir::UnitAttr::get(&getMLIRContext()));
157+
158+
if (!emission.wasEmittedAsOffloadClause()) {
159+
// In case lv has uses it means we indeed initialized something
160+
// out of it while trying to build the expression, mark it as such.
161+
mlir::Value val = lv.getAddress().getPointer();
162+
assert(val && "Should have an address");
163+
auto allocaOp = val.getDefiningOp<cir::AllocaOp>();
164+
assert(allocaOp && "Address should come straight out of the alloca");
165+
166+
if (!allocaOp.use_empty())
167+
allocaOp.setInitAttr(mlir::UnitAttr::get(&getMLIRContext()));
168+
}
169+
166170
return;
167171
}
168172

clang/lib/CIR/CodeGen/CIRGenFunction.h

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,10 @@ class CIRGenFunction : public CIRGenTypeCache {
471471
/// escaping block.
472472
bool IsEscapingByRef = false;
473473

474+
/// True if the variable was emitted as an offload recipe, and thus doesn't
475+
/// have the same sort of alloca initialization.
476+
bool EmittedAsOffload = false;
477+
474478
mlir::Value NRVOFlag{};
475479

476480
struct Invalid {};
@@ -483,11 +487,18 @@ class CIRGenFunction : public CIRGenTypeCache {
483487

484488
bool wasEmittedAsGlobal() const { return !Addr.isValid(); }
485489

490+
bool wasEmittedAsOffloadClause() const { return EmittedAsOffload; }
491+
486492
/// Returns the raw, allocated address, which is not necessarily
487493
/// the address of the object itself. It is casted to default
488494
/// address space for address space agnostic languages.
489495
Address getAllocatedAddress() const { return Addr; }
490496

497+
// Changes the stored address for the emission. This function should only
498+
// be used in extreme cases, and isn't required to model normal AST
499+
// initialization/variables.
500+
void setAllocatedAddress(Address A) { Addr = A; }
501+
491502
/// Returns the address of the object within this declaration.
492503
/// Note that this does not chase the forwarding pointer for
493504
/// __block decls.
@@ -519,12 +530,34 @@ class CIRGenFunction : public CIRGenTypeCache {
519530
symbolTable.insert(vd, addr.getPointer());
520531
}
521532

522-
/// Removes a declaration from the address-relationship. This is a function
523-
/// that shouldn't need to be used except in cases where we're adding/removing
524-
/// things that aren't part of the language-semantics AST.
525-
void removeAddrOfLocalVar(const clang::VarDecl *vd) {
526-
localDeclMap.erase(vd);
527-
}
533+
// A class to allow reverting changes to a var-decl's registration to the
534+
// localDeclMap. This is used in cases where things are being inserted into
535+
// the variable list but don't follow normal lookup/search rules, like in
536+
// OpenACC recipe generation.
537+
class DeclMapRevertingRAII {
538+
CIRGenFunction &cgf;
539+
const VarDecl *vd;
540+
bool shouldDelete = false;
541+
Address oldAddr = Address::invalid();
542+
543+
public:
544+
DeclMapRevertingRAII(CIRGenFunction &cgf, const VarDecl *vd)
545+
: cgf(cgf), vd(vd) {
546+
auto mapItr = cgf.localDeclMap.find(vd);
547+
548+
if (mapItr != cgf.localDeclMap.end())
549+
oldAddr = mapItr->second;
550+
else
551+
shouldDelete = true;
552+
}
553+
554+
~DeclMapRevertingRAII() {
555+
if (shouldDelete)
556+
cgf.localDeclMap.erase(vd);
557+
else
558+
cgf.localDeclMap.insert_or_assign(vd, oldAddr);
559+
}
560+
};
528561

529562
bool shouldNullCheckClassCastValue(const CastExpr *ce);
530563

@@ -970,6 +1003,11 @@ class CIRGenFunction : public CIRGenTypeCache {
9701003
void emitAutoVarDecl(const clang::VarDecl &d);
9711004

9721005
void emitAutoVarCleanups(const AutoVarEmission &emission);
1006+
/// Emit the initializer for an allocated variable. If this call is not
1007+
/// associated with the call to emitAutoVarAlloca (as the address of the
1008+
/// emission is not directly an alloca), the allocatedSeparately parameter can
1009+
/// be used to suppress the assertions. However, this should only be used in
1010+
/// extreme cases, as it doesn't properly reflect the language/AST.
9731011
void emitAutoVarInit(const AutoVarEmission &emission);
9741012
void emitAutoVarTypeCleanup(const AutoVarEmission &emission,
9751013
clang::QualType::DestructionKind dtorKind);

clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -392,12 +392,29 @@ class OpenACCClauseCIREmitter final
392392
CIRGenFunction::AutoVarEmission tempDeclEmission,
393393
mlir::acc::FirstprivateRecipeOp recipe, const VarDecl *varRecipe,
394394
const VarDecl *temporary) {
395-
builder.createBlock(&recipe.getCopyRegion(), recipe.getCopyRegion().end(),
396-
{mainOp.getType(), mainOp.getType()}, {loc, loc});
395+
mlir::Block *block = builder.createBlock(
396+
&recipe.getCopyRegion(), recipe.getCopyRegion().end(),
397+
{mainOp.getType(), mainOp.getType()}, {loc, loc});
397398
builder.setInsertionPointToEnd(&recipe.getCopyRegion().back());
398399

399-
// TODO: OpenACC: Implement this copy to actually do something.
400+
mlir::BlockArgument fromArg = block->getArgument(0);
401+
mlir::BlockArgument toArg = block->getArgument(1);
402+
403+
mlir::Type elementTy =
404+
mlir::cast<cir::PointerType>(mainOp.getType()).getPointee();
405+
406+
// Set the address of the emission to be the argument, so that we initialize
407+
// that instead of the variable in the other block.
408+
tempDeclEmission.setAllocatedAddress(
409+
Address{toArg, elementTy, cgf.getContext().getDeclAlign(varRecipe)});
410+
tempDeclEmission.EmittedAsOffload = true;
411+
412+
CIRGenFunction::DeclMapRevertingRAII declMapRAII{cgf, temporary};
413+
cgf.setAddrOfLocalVar(
414+
temporary,
415+
Address{fromArg, elementTy, cgf.getContext().getDeclAlign(varRecipe)});
400416

417+
cgf.emitAutoVarInit(tempDeclEmission);
401418
mlir::acc::YieldOp::create(builder, locEnd);
402419
}
403420

@@ -417,6 +434,7 @@ class OpenACCClauseCIREmitter final
417434

418435
CIRGenFunction::AutoVarEmission tempDeclEmission{
419436
CIRGenFunction::AutoVarEmission::invalid()};
437+
CIRGenFunction::DeclMapRevertingRAII declMapRAII{cgf, varRecipe};
420438

421439
// Do the 'init' section of the recipe IR, which does an alloca, then the
422440
// initialization (except for firstprivate).
@@ -425,6 +443,7 @@ class OpenACCClauseCIREmitter final
425443
builder.setInsertionPointToEnd(&recipe.getInitRegion().back());
426444
tempDeclEmission =
427445
cgf.emitAutoVarAlloca(*varRecipe, builder.saveInsertionPoint());
446+
428447
// 'firstprivate' doesn't do its initialization in the 'init' section,
429448
// instead does it in the 'copy' section. SO only do init here.
430449
// 'reduction' appears to use it too (rather than a 'copy' section), so
@@ -450,16 +469,19 @@ class OpenACCClauseCIREmitter final
450469
mlir::acc::YieldOp::create(builder, locEnd);
451470

452471
if constexpr (std::is_same_v<RecipeTy, mlir::acc::FirstprivateRecipeOp>) {
453-
// TODO: OpenACC: we should have a errorNYI call here if
454-
// !varRecipe->getInit(), but as that generation isn't currently
455-
// implemented, it ends up being too noisy. So when we implement copy-init
456-
// generation both in Sema and here, we should have a diagnostic here.
472+
if (!varRecipe->getInit()) {
473+
// If we don't have any initialization recipe, we failed during Sema to
474+
// initialize this correctly. If we disable the
475+
// Sema::TentativeAnalysisScopes in SemaOpenACC::CreateInitRecipe, it'll
476+
// emit an error to tell us. However, emitting those errors during
477+
// production is a violation of the standard, so we cannot do them.
478+
cgf.cgm.errorNYI(
479+
exprRange, "firstprivate copy-init recipe not properly generated");
480+
}
481+
457482
createFirstprivateRecipeCopy(loc, locEnd, mainOp, tempDeclEmission,
458483
recipe, varRecipe, temporary);
459484
}
460-
461-
// Make sure we cleanup after ourselves here.
462-
cgf.removeAddrOfLocalVar(varRecipe);
463485
}
464486

465487
void createRecipeDestroySection(mlir::Location loc, mlir::Location locEnd,

0 commit comments

Comments
 (0)