ABataev added inline comments.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2566-2569
+ if (*Res == OMPDeclareTargetDeclAttr::MT_Link)
+ OS << CGM.getMangledName(GlobalDecl(VD)) << "_decl_tgt_link_ptr";
+ else
+ OS << CGM.getMangledName(GlobalDecl(VD)) << "_decl_tgt_to_ptr";
----------------
I think we can use the same suffix in all cases, instead of `_link_ptr` or
`_to_ptr` just something like `_ref_ptr` would be good.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7465
bool IsCaptureFirstInfo = IsFirstComponentList;
- bool IsLink = false; // Is this variable a "declare target link"?
+ bool IsLinkOrToClause = false; // Is this variable a "declare target link"?
----------------
Fix the comment too. Also, the name of the variable is not very good, better to
rename it to something like `MustBeReference` or something similar.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9230-9232
Flags = OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryLink;
+ if (*Res == OMPDeclareTargetDeclAttr::MT_To)
+ Flags = OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryTo;
----------------
Better to have `if-else` here
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9270
+ assert((*Res == OMPDeclareTargetDeclAttr::MT_Link ||
+ *Res == OMPDeclareTargetDeclAttr::MT_To) &&
"Expected to or link clauses.");
----------------
`|| (*Res == OMPDeclareTargetDeclAttr::MT_To &&
HasRequiresUnifiedSharedMemory)` and fix the message too
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:1125
+ /// clause OR as declare target with to clause and unified memory.
+ virtual Address getAddrOfDeclareTargetClause(const VarDecl *VD);
----------------
Better to name it `getAddrOfDeclareTargetVar`
================
Comment at: lib/CodeGen/CodeGenModule.cpp:2483-2485
+ else if (*Res == OMPDeclareTargetDeclAttr::MT_Link ||
+ (*Res == OMPDeclareTargetDeclAttr::MT_To &&
+ UnifiedMemoryEnabled))
----------------
Better to keep the assert but with a different condition.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63108/new/
https://reviews.llvm.org/D63108
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
