ABataev added a comment.

In D144634#4308872 <https://reviews.llvm.org/D144634#4308872>, @koops wrote:

> 1. Taken care of some of the comments by Alexey.
> 2. Added extra tests of loop_bind_messages.cpp
>
> However, generic_loop_messages.cpp & generic_loop_ast_print.cpp are present 
> and provide a good coverage.

I rather doubt that these tests provide good coverage, since you're changing 
the directive kind here on the fly. This is a very new functionality, which was 
not tested before.
Add the tests for nesting of the regions, i.e. loop bind directive, enclosed in 
different regions, and diagnostics.



================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:339
 
+  OpenMPDirectiveKind Map1D = OMPD_unknown;
+
----------------
What Ma1D stands for, what is the meaning of this abbrev? Add a comment.


================
Comment at: clang/test/OpenMP/loop_bind_codegen.cpp:2
+// expected-no-diagnostics
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -x c++ %s -emit-llvm -o - | 
FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown 
-emit-llvm %s -o - | FileCheck %s
----------------
Add tests with save/load precompiled code.


================
Comment at: clang/test/OpenMP/loop_bind_messages.cpp:3
+#define HEADER
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -verify %s
+
----------------
Add tests with save/load precompiled code.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144634/new/

https://reviews.llvm.org/D144634

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to