jdoerfert marked 5 inline comments as done.
jdoerfert added inline comments.
================
Comment at: clang/include/clang/Basic/OpenMPKinds.h:29
- OMPD_unknown
+ OMPD_unknown,
};
----------------
These changes do not impact Clang nor do they remove functionality (if
OPENMP_DIRECTIVE is defined it will work as it did before) but they are
necessary to make the enum values in Clang and the OpenMPIRBuilder equal.
The reason is that the OPENMP_DIRECTIVE and OPENMP_DIRECTIVE_EXT definitions in
`OpenMPKinds.def` are interleaved but for the OpenMPIRBuilder I was hoping not
to separate them logically. So the first three changes adjust Clang towards
this simpler model that is still as powerful as the old one was but which
changes the order of the enums.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3490
+ }
+
if (!CGF.HaveInsertPoint())
----------------
The first extension could be for Clang to communicate the cancellation blocks
to the OMPBuilder which would allow us there to handle all barriers. Note:
There is no need for a callback (in the long run) as cancellation points will
be handled by the OMPBuilder eventually.
================
Comment at: clang/test/OpenMP/for_firstprivate_codegen.cpp:291
// CHECK: define internal void [[TMAIN_MICROTASK]](i{{[0-9]+}}* noalias
[[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i32* dereferenceable(4)
%{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [2 x [[S_INT_TY]]]*
dereferenceable(8) %{{.+}}, [[S_INT_TY]]* dereferenceable(4) %{{.+}})
+// CHECK: [[GTID_CALL:%.+]] = call i32 @__kmpc_global_thread_num
// Skip temp vars for loop
----------------
All test changes are because we do not check if there is an existing global
thread ID value available through an argument. See the comment in
`OpenMPIRBuilder::getThreadID` for further information and a proper way out.
================
Comment at: llvm/include/llvm/IR/OpenMPIRBuilder.h:36
+#include "llvm/IR/OpenMPKinds.def"
+ };
+
----------------
The enum values are equivalent to the ones in Clangs `OpenMPDirectiveKind`,
they have to be as we convert the latter to the former right now. At some point
we won't need the latter but for now we should probably put a static assert
here to verify the first and last enum values are equal.
================
Comment at: llvm/include/llvm/IR/OpenMPIRBuilder.h:58
+ IRBuilder<>::InsertPoint IP;
+ };
+
----------------
This struct needs to be extended with source location information in a way not
tied to Clang. We wan to use that for two purposes: (1) generate `ident_t` with
actual source locations embedded and (2) tell the IRBuilder to emit debug info
if requested.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69785/new/
https://reviews.llvm.org/D69785
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
