[PATCH] D132855: [OpenMP] Extend the lit test for uses_allocators in target region

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LG



================
Comment at: clang/test/OpenMP/target_map_codegen_10.cpp:19
+// RUN: %clang_cc1 -no-opaque-pointers -DCK11 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  %s  --check-prefixes CK11,CK11_5
 
----------------
animeshk-amd wrote:
> jdoerfert wrote:
> > What does this test?
> This test was originally testing the respective directives only for the OpenMP version 4.5. I have updated new RUN lines for the default version(5.0 as of now) because there were differences in the generated IR in case of the default version.
I see. Makes sense.


================
Comment at: clang/test/OpenMP/target_uses_allocators.c:42
+  #pragma omp target uses_allocators(omp_thread_mem_alloc) allocate(omp_thread_mem_alloc: x) firstprivate(x) // expected-warning {{allocator with the 'thread' trait access has unspecified behavior on 'target' directive}}
+  {}
 }
----------------
animeshk-amd wrote:
> jdoerfert wrote:
> > This should go into the _messages test case
> The code needs to be tested for the generated IR(but as a side effect it also generates a warning), that's why I had to put the expected-warning clause. Since I had to write CHECK lines, I didn't put this in the _messages test. That said, should I still put this into the _messages test along with the required CHECK lines?
I am not really sure about the warning to begin with but I get your point about placement.
I'm ok with this for now and we revisit this as the next OpenMP TR is going to clarify the allocate traits anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132855