[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder
Alexey Bataev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 4 08:07:46 PST 2019
ABataev added inline comments.
================
Comment at: clang/include/clang/Basic/OpenMPKinds.h:23-24
enum OpenMPDirectiveKind {
-#define OPENMP_DIRECTIVE(Name) \
- OMPD_##Name,
#define OPENMP_DIRECTIVE_EXT(Name, Str) \
----------------
Better to make in a separate NFC patch.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3483
+ auto *OMPRegionInfo = dyn_cast_or_null<CGOpenMPRegionInfo>(CGF.CapturedStmtInfo);
+ if (!EmitChecks || ForceSimpleCall || !OMPRegionInfo ||
+ !OMPRegionInfo->hasCancel()) {
----------------
I would add an option, something like `-fopenmp-experimental` for all changes with OpenMPBulder unless it at least matches in functionality/performance with the existing solution.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3486
+ CGF.CGM.getOpenMPIRBuilder().emitOMPBarrier(
+ {CGF.Builder.saveIP()}, llvm::OpenMPIRBuilder::DirektiveKind(Kind),
+ EmitChecks);
----------------
`DirectiveKind`
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3493
return;
+
// Build call __kmpc_cancel_barrier(loc, thread_id);
----------------
Better to remove all these formatting changes from the patch.
================
Comment at: llvm/include/llvm/IR/OpenMPIRBuilder.h:33
+ /// IDs for all OpenMP directives.
+ enum DirektiveKind {
+#define OMP_DIRECTIVE(Enum, ...) Enum,
----------------
1. `DirectiveKind`
2. Is it possible to merge these 2 types in LLVM and in clang into 1?
================
Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:108
+ DefaultIdent->setUnnamedAddr(GlobalValue::UnnamedAddr::Global);
+ DefaultIdent->setAlignment(Align(8));
+ }
----------------
Is this correct for 32-bit platforms? Maybe rely on the frontend when creating structures and all other required feature things?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69785/new/
https://reviews.llvm.org/D69785
More information about the cfe-commits
mailing list