-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[OpenACC] Implement 'firstprivate' clause copy lowering #154150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,7 +77,7 @@ bool CIRGenFunction::isTrivialInitializer(const Expr *init) { | |
| } | ||
|
|
||
| void CIRGenFunction::emitAutoVarInit( | ||
| const CIRGenFunction::AutoVarEmission &emission) { | ||
| const CIRGenFunction::AutoVarEmission &emission, bool allocatedSeparately) { | ||
| assert(emission.Variable && "emission was not valid!"); | ||
|
|
||
| // If this was emitted as a global constant, we're done. | ||
|
|
@@ -159,9 +159,10 @@ void CIRGenFunction::emitAutoVarInit( | |
| mlir::Value val = lv.getAddress().getPointer(); | ||
| assert(val && "Should have an address"); | ||
| auto allocaOp = val.getDefiningOp<cir::AllocaOp>(); | ||
| assert(allocaOp && "Address should come straight out of the alloca"); | ||
| assert((allocatedSeparately || allocaOp) && | ||
| "Address should come straight out of the alloca"); | ||
|
|
||
| if (!allocaOp.use_empty()) | ||
| if (allocaOp && !allocaOp.use_empty()) | ||
|
||
| allocaOp.setInitAttr(mlir::UnitAttr::get(&getMLIRContext())); | ||
| return; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -487,6 +487,11 @@ class CIRGenFunction : public CIRGenTypeCache { | |
| /// address space for address space agnostic languages. | ||
| Address getAllocatedAddress() const { return Addr; } | ||
|
|
||
| // Changes the stored address for the emission. This function should only | ||
| // be used in extreme cases, and isn't required to model normal AST | ||
| // initialization/variables. | ||
| void setAllocatedAddress(Address A) { Addr = A; } | ||
|
|
||
| /// Returns the address of the object within this declaration. | ||
| /// Note that this does not chase the forwarding pointer for | ||
| /// __block decls. | ||
|
|
@@ -518,12 +523,19 @@ class CIRGenFunction : public CIRGenTypeCache { | |
| symbolTable.insert(vd, addr.getPointer()); | ||
| } | ||
|
|
||
| /// Removes a declaration from the address-relationship. This is a function | ||
| /// that shouldn't need to be used except in cases where we're adding/removing | ||
| /// things that aren't part of the language-semantics AST. | ||
| void removeAddrOfLocalVar(const clang::VarDecl *vd) { | ||
| localDeclMap.erase(vd); | ||
| } | ||
| // A class to allow inserting things into the declaration map during some sort | ||
| // of alternative generation (used currently for the OpenACC recipe | ||
| // generation), then reverting changes after the fact. | ||
| class DeclMapRevertingRAII { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to have a similar purpose to In the uses here, do you expect any addresses to be added to the map other than clause parameters?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really expect ANYTHING other than the clause parameters, but it seemed less error prone than making the add/remove the same operation. That said, I can probably make this 'less intrusive' than a copy + move by making me just record the decl that I'd like to replace the value of.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could live with it either way, but it would be nice if we could somehow combine this with the coroutine parameter handling when that gets upstreamed.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ping me when that gets to review, and I'll work with the person to combine them. At the moment, I'm a bit too 'simple' for that, but when that goes under review I'll work with the uathor to combine the uses. |
||
| CIRGenFunction &cgf; | ||
| CIRGenFunction::DeclMapTy originalMap; | ||
|
|
||
| public: | ||
| DeclMapRevertingRAII(CIRGenFunction &cgf) | ||
| : cgf(cgf), originalMap(cgf.localDeclMap) {} | ||
|
|
||
| ~DeclMapRevertingRAII() { cgf.localDeclMap = std::move(originalMap); } | ||
| }; | ||
|
|
||
| bool shouldNullCheckClassCastValue(const CastExpr *ce); | ||
|
|
||
|
|
@@ -961,7 +973,13 @@ class CIRGenFunction : public CIRGenTypeCache { | |
| void emitAutoVarDecl(const clang::VarDecl &d); | ||
|
|
||
| void emitAutoVarCleanups(const AutoVarEmission &emission); | ||
| void emitAutoVarInit(const AutoVarEmission &emission); | ||
| /// Emit the initializer for an allocated variable. If this call is not | ||
| /// associated with the call to emitAutoVarAlloca (as the address of the | ||
| /// emission is not directly an alloca), the allocatedSeparately parameter can | ||
| /// be used to suppress the assertions. However, this should only be used in | ||
| /// extreme cases, as it doesn't properly reflect the language/AST. | ||
| void emitAutoVarInit(const AutoVarEmission &emission, | ||
| bool allocatedSeparately = false); | ||
| void emitAutoVarTypeCleanup(const AutoVarEmission &emission, | ||
| clang::QualType::DestructionKind dtorKind); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than passing this in as an argument here, could you make it a member of AutoVarEmission?