Skip to content

Commit 478331c

Browse files
committed
[OpenACC] Implement 'firstprivate' clause copy lowering
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 ec22705 commit 478331c

File tree

6 files changed

+492
-32
lines changed

6 files changed

+492
-32
lines changed

clang/lib/CIR/CodeGen/CIRGenDecl.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ bool CIRGenFunction::isTrivialInitializer(const Expr *init) {
7777
}
7878

7979
void CIRGenFunction::emitAutoVarInit(
80-
const CIRGenFunction::AutoVarEmission &emission) {
80+
const CIRGenFunction::AutoVarEmission &emission, bool allocatedSeparately) {
8181
assert(emission.Variable && "emission was not valid!");
8282

8383
// If this was emitted as a global constant, we're done.
@@ -159,9 +159,10 @@ void CIRGenFunction::emitAutoVarInit(
159159
mlir::Value val = lv.getAddress().getPointer();
160160
assert(val && "Should have an address");
161161
auto allocaOp = val.getDefiningOp<cir::AllocaOp>();
162-
assert(allocaOp && "Address should come straight out of the alloca");
162+
assert((allocatedSeparately || allocaOp) &&
163+
"Address should come straight out of the alloca");
163164

164-
if (!allocaOp.use_empty())
165+
if (allocaOp && !allocaOp.use_empty())
165166
allocaOp.setInitAttr(mlir::UnitAttr::get(&getMLIRContext()));
166167
return;
167168
}

clang/lib/CIR/CodeGen/CIRGenFunction.h

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,11 @@ class CIRGenFunction : public CIRGenTypeCache {
487487
/// address space for address space agnostic languages.
488488
Address getAllocatedAddress() const { return Addr; }
489489

490+
// Changes the stored address for the emission. This function should only
491+
// be used in extreme cases, and isn't required to model normal AST
492+
// initialization/variables.
493+
void setAllocatedAddress(Address A) { Addr = A; }
494+
490495
/// Returns the address of the object within this declaration.
491496
/// Note that this does not chase the forwarding pointer for
492497
/// __block decls.
@@ -518,12 +523,19 @@ class CIRGenFunction : public CIRGenTypeCache {
518523
symbolTable.insert(vd, addr.getPointer());
519524
}
520525

521-
/// Removes a declaration from the address-relationship. This is a function
522-
/// that shouldn't need to be used except in cases where we're adding/removing
523-
/// things that aren't part of the language-semantics AST.
524-
void removeAddrOfLocalVar(const clang::VarDecl *vd) {
525-
localDeclMap.erase(vd);
526-
}
526+
// A class to allow inserting things into the declaration map during some sort
527+
// of alternative generation (used currently for the OpenACC recipe
528+
// generation), then reverting changes after the fact.
529+
class DeclMapRevertingRAII {
530+
CIRGenFunction &cgf;
531+
CIRGenFunction::DeclMapTy originalMap;
532+
533+
public:
534+
DeclMapRevertingRAII(CIRGenFunction &cgf)
535+
: cgf(cgf), originalMap(cgf.localDeclMap) {}
536+
537+
~DeclMapRevertingRAII() { cgf.localDeclMap = std::move(originalMap); }
538+
};
527539

528540
bool shouldNullCheckClassCastValue(const CastExpr *ce);
529541

@@ -961,7 +973,13 @@ class CIRGenFunction : public CIRGenTypeCache {
961973
void emitAutoVarDecl(const clang::VarDecl &d);
962974

963975
void emitAutoVarCleanups(const AutoVarEmission &emission);
964-
void emitAutoVarInit(const AutoVarEmission &emission);
976+
/// Emit the initializer for an allocated variable. If this call is not
977+
/// associated with the call to emitAutoVarAlloca (as the address of the
978+
/// emission is not directly an alloca), the allocatedSeparately parameter can
979+
/// be used to suppress the assertions. However, this should only be used in
980+
/// extreme cases, as it doesn't properly reflect the language/AST.
981+
void emitAutoVarInit(const AutoVarEmission &emission,
982+
bool allocatedSeparately = false);
965983
void emitAutoVarTypeCleanup(const AutoVarEmission &emission,
966984
clang::QualType::DestructionKind dtorKind);
967985

clang/lib/CIR/CodeGen/CIRGenOpenACCClause.cpp

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -392,12 +392,27 @@ 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+
411+
cgf.setAddrOfLocalVar(
412+
temporary,
413+
Address{fromArg, elementTy, cgf.getContext().getDeclAlign(varRecipe)});
400414

415+
cgf.emitAutoVarInit(tempDeclEmission, /*allocatedSeparately=*/true);
401416
mlir::acc::YieldOp::create(builder, locEnd);
402417
}
403418

@@ -417,6 +432,7 @@ class OpenACCClauseCIREmitter final
417432

418433
CIRGenFunction::AutoVarEmission tempDeclEmission{
419434
CIRGenFunction::AutoVarEmission::invalid()};
435+
CIRGenFunction::DeclMapRevertingRAII declMapRAII{cgf};
420436

421437
// Do the 'init' section of the recipe IR, which does an alloca, then the
422438
// initialization (except for firstprivate).
@@ -425,6 +441,7 @@ class OpenACCClauseCIREmitter final
425441
builder.setInsertionPointToEnd(&recipe.getInitRegion().back());
426442
tempDeclEmission =
427443
cgf.emitAutoVarAlloca(*varRecipe, builder.saveInsertionPoint());
444+
428445
// 'firstprivate' doesn't do its initialization in the 'init' section,
429446
// instead does it in the 'copy' section. SO only do init here.
430447
// 'reduction' appears to use it too (rather than a 'copy' section), so
@@ -450,16 +467,19 @@ class OpenACCClauseCIREmitter final
450467
mlir::acc::YieldOp::create(builder, locEnd);
451468

452469
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.
470+
if (!varRecipe->getInit()) {
471+
// If we don't have any initialization recipe, we failed during Sema to
472+
// initialize this correctly. If we disable the
473+
// Sema::TentativeAnalysisScopes in SemaOpenACC::CreateInitRecipe, it'll
474+
// emit an error to tell us. However, emitting those errors during
475+
// production is a violation of the standard, so we cannot do them.
476+
cgf.cgm.errorNYI(
477+
exprRange, "firstprivate copy-init recipe not properly generated");
478+
}
479+
457480
createFirstprivateRecipeCopy(loc, locEnd, mainOp, tempDeclEmission,
458481
recipe, varRecipe, temporary);
459482
}
460-
461-
// Make sure we cleanup after ourselves here.
462-
cgf.removeAddrOfLocalVar(varRecipe);
463483
}
464484

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

0 commit comments

Comments
 (0)