jdoerfert added a comment.
Generally, this is fine. Some nits. The only reason not to accept this right
now is the test. Why manual check lines?
Wrt. the function signature. As soon as we have more then SIMD directives to
check we refactor things. Keep it simple
until you need the functionality.
================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2598
+ // Currently only simdlen clause is supported
+ if (!dyn_cast<OMPSimdlenClause>(C))
+ return false;
----------------
================
Comment at: clang/test/OpenMP/irbuilder_simdlen.cpp:1
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-enable-irbuilder -verify
-fopenmp -fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm
%s -o - | FileCheck %s
+// expected-no-diagnostics
----------------
The check lines look auto-generated and then modified by hand. Why is that?
================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:608
+ ///
+ /// \param DL Debug location for instructions added by unrolling.
+ /// \param Loop The simd loop.
----------------
No debug location needed. You also copied the comment that makes little sene.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2885
+void OpenMPIRBuilder::applySimdlen(DebugLoc, CanonicalLoopInfo *CanonicalLoop,
+ llvm::ConstantInt *Simdlen) {
+ LLVMContext &Ctx = Builder.getContext();
----------------
Also above, no llvm::
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129149/new/
https://reviews.llvm.org/D129149
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
