search

OpenMPToolsInterface / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies. Note: the repository does not accept github pull requests at this moment. Please submit your patches at http://reviews.llvm.org.
http://llvm.org
2 stars 4 forks source link

Infra to test OMPD APIs, this patch focuses more on return codes #25

Closed vigbalu closed 3 years ago

vigbalu commented 3 years ago

Currently, we don't have infra in OMPD to validate API individually. The available testing is to verify through “ompd gdb commands” which is not compatible for a few validations like checking “return codes” with invalid arguments. This patch will add more “gdb” commands (one per API, can be made hidden) and the user can test the API individually through the same with the proper test case. API with a certain combination of invalid arguments is crashing. These are skipped and will enable once it is fixed. The next patch will include the remaining APIs and then fixes of failed tests.

jprotze commented 3 years ago

For upstreaming OMPD we will need to run clang-format on all the OMPD code. LLVM style says indent by 2 spaces, limit line width to 80 (clang-format will take care of this). This will break your super wide comments in ompdAPITests.c. So, I suggest that you run git clang-format HEAD~2 on your branch (only format the modifications of the last two commits) and repair the comments.

An annoying side-effect of clang-format is that it breaks also comments with lit commands. Can we reduce the width of the RUN lines like:

// RUN: %gdb-test -x %S/test_ompd_get_scheduling_task_handle.cmd %t 2>&1 | tee %t.out | FileCheck %s

change the RUN line to (should work, but is not obvious):

// RUN: %gdb-test -x %smd %t 2>&1 | tee %t.out | FileCheck %s

or move test_ompd_get_scheduling_task_handle.cmd to test_ompd_get_scheduling_task_handle.c.cmd and change the RUN line to:

// RUN: %gdb-test -x %s.cmd %t 2>&1 | tee %t.out | FileCheck %s

For OMPT, the tee is part of the FileCheck replacement, which can safe extra space: https://github.com/OpenMPToolsInterface/llvm-project/blob/ompd-tests/openmp/runtime/test/lit.cfg#L134

vigbalu commented 3 years ago

Addressed the review comments in the below patch. https://github.com/vigbalu/llvm-project/commit/335ec73a62bb81aa3c2c19ee0c083b179dd06a20

jprotze commented 3 years ago

Thanks! I hope I got the merge right.