[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

protze.joachim reopened this revision. protze.joachim added a comment. This revision is now accepted and ready to land. The review should not be closed, since the patch was reverted and should be recommitted (this time possibly with a reference to this review?) Repository: rG LLVM Github Mono

[PATCH] D131830: [OpenMP] Clang Support for taskwait nowait clause

protze.joachim added a comment. Why do we need __kmpc_omp_taskwait_51 ? Taskwait nowait only makes sense with dependencies. It should only result in additional nodes/edges in the task dependency graph without any code to execute. There is not taskwait (aka task barrier) in this case. The taskw

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

protze.joachim added a comment. I think in openmp/tools currently only libraries and header files get installed, but no binaries. I suggest to define `OPENMP_TOOLS_INSTALL_BINDIR`, `OPENMP_TOOLS_INSTALL_LIBDIR`, and `OPENMP_TOOLS_INSTALL_INCLUDEDIR` and probably base these variables on OPENMP_

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

protze.joachim added a comment. In D101960#2960641 , @JonChesterfield wrote: > Pasting `env LD_LIBRARY_PATH=` and `env LIBRARY_PATH` into the printed > commandline, as opposed to through magic, would solve most of my day to day > frustration here. libo

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

protze.joachim added a comment. In D106509#2943316 , @grokos wrote: > The next patch in this series (D106510 ) > modifies libomptarget and introduces a second reference count for ompx_hold. > There won't be a singe RefC

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

protze.joachim added a comment. I was wondering about the connection to OpenACC, so I had a quick look into the OpenACC spec to try and understand the background. OpenACC uses two separate reference counters for structured and unstructured map. If one of them is >0, the data is present. If both

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

protze.joachim added a comment. To make debugging of failed tests easier, we could just explicitly include `env LD_LIBRARY_PATH=...` into each run line - by adding it to the `%libomp-run` substitution (and the libomptarget equivalent). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D99353: [driver] Make `clang` warn rather then error on `flang` options

protze.joachim added a comment. Since I see this as a regression over clang-12, I posted https://bugs.llvm.org/show_bug.cgi?id=51339 as a release blocking issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99353/new/ https://reviews.llvm.org/D99

[PATCH] D99353: [driver] Make `clang` warn rather then error on `flang` options

protze.joachim added a comment. The use case, where I see the current behavior as an issue is linking multiple object files compiled from different languages. Many build systems will pass CFLAGS as well as FFLAGS to the linker command in such case. To automatically link the necessary language-sp

[PATCH] D99353: [driver] Make `clang` warn rather then error on `flang` options

protze.joachim added a comment. Any chance that we get this into llvm/13? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99353/new/ https://reviews.llvm.org/D99353 ___ cfe-commits mailing list cfe-commits

[PATCH] D97085: [OpenMP] libomp: implement OpenMP 5.1 inoutset task dependence type

protze.joachim added inline comments. Comment at: openmp/runtime/src/kmp_taskdeps.cpp:418 if (dep_list[i].base_addr != 0) { + KMP_DEBUG_ASSERT( + dep_list[i].flag == KMP_DEP_IN || dep_list[i].flag == KMP_DEP_OUT || I hit this assertion, when co

[PATCH] D103885: [clang] Suppress warnings for tautological comparison in generated macro code

protze.joachim added a comment. I don't think, this needs ifdefs. As I understand, a compiler is supposed to ignore unknown pragmas. I fully agree that the suppression will not be compatible with other compilers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D103885: [clang] Suppress warnings for tautological comparison in generated macro code

protze.joachim created this revision. protze.joachim added reviewers: jdoerfert, aaron.ballman, hokein. protze.joachim added projects: clang, clang-tools-extra. Herald added subscribers: usaxena95, kadircet, arphaman, hiraditya. protze.joachim requested review of this revision. Herald added subscri

[PATCH] D97085: [OpenMP] libomp: implement OpenMP 5.1 inoutset task dependence type

protze.joachim added inline comments. Comment at: openmp/runtime/src/kmp_taskdeps.cpp:344-346 +// link node as successor of all nodes in the prev_set if any +npredecessors += +__kmp_depnode_link_successor(gtid, thread, task, node, prev_set); --

[PATCH] D97085: [OpenMP] libomp: implement OpenMP 5.1 inoutset task dependence type

protze.joachim added inline comments. Comment at: openmp/runtime/src/kmp_taskdeps.cpp:344-346 +// link node as successor of all nodes in the prev_set if any +npredecessors += +__kmp_depnode_link_successor(gtid, thread, task, node, prev_set); --

[PATCH] D101960: [openmp] Drop requirement on library path environment variables

protze.joachim added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:648 +void tools::addOpenMPRuntimeSpecificRPath(const ToolChain &TC, + const ArgList &Args, JonChesterfield wrote: > lebedev.ri

[PATCH] D95976: [OpenMP] Simplify offloading parallel call codegen

protze.joachim added a comment. Please update the test with a NFC commit. Comment at: openmp/libomptarget/test/offloading/bug49779.cpp:1-5 +// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu +// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

protze.joachim accepted this revision. protze.joachim added a comment. This revision is now accepted and ready to land. The latest changes fix the issue. I tested with x86_64 and nvidia offloading. The behavior and tests looks good to me, just one comment. Comment at: openmp/

[PATCH] D100673: [OPENMP]Fix PR49698: OpenMP declare mapper causes segmentation fault.

protze.joachim requested changes to this revision. protze.joachim added a comment. This revision now requires changes to proceed. I tested the patch and still get wrong results (I modified the `declare_mapper_nested_default_mappers_complex_structure.cpp` test to use my verbose printf from bugzil

[PATCH] D99353: [driver] Make `clang` warn rather then error on `flang` options

protze.joachim added a comment. In D99353#2669046 , @awarzynski wrote: > Btw, how important are these aliases for you? I can work with `-ffixed-line-length=132` where needed. It's just not obvious from `flang --help` that this is an alias for `-ffixed-li

[PATCH] D99353: [driver] Make `clang` warn rather then error on `flang` options

protze.joachim added a comment. Thank you for working on this! It works for me. As you mentioned in D95460 , this makes the behavior more similar to gcc. I tested with `-Werror`: $ flang -fopenmp test-f77.f -ffree-form -c $ clang -fopenmp test-f77.o -ffree-f

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

protze.joachim added a comment. Hi Andrzej, thanks for the detailed insights. I think I really misunderstood, what the -W flags can do here. I also was not up-to-date regarding the state of the flang implementation. I just found, that compiling Spec OMP 2012 with clang-12 worked fine with my

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

protze.joachim added a comment. Before this patch, clang would happily ignore a `-ffree-form` flag. With this patch, none of `-Wno-error=unknown-argument`, `-Wno-unknown-argument` , `-Qunused-arguments` help to avoid clang from exiting with error: unknown argument: '-ffree-form' Why can't cl

[PATCH] D97085: [OpenMP] libomp: implement OpenMP 5.1 inoutset task dependence type

protze.joachim added inline comments. Comment at: openmp/runtime/src/kmp_taskdeps.cpp:344-346 +// link node as successor of all nodes in the prev_set if any +npredecessors += +__kmp_depnode_link_successor(gtid, thread, task, node, prev_set); --

[PATCH] D97085: [OpenMP] libomp: implement OpenMP 5.1 inoutset task dependence type

protze.joachim added a comment. Can you add the inoutset case to the ompt code as well? omp-tools.h already defines `ompt_dependence_type_inoutset`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97085/new/ https://reviews.llvm.org/D97085

[PATCH] D91944: OpenMP 5.0 metadirective

protze.joachim added a comment. In D91944#2458154 , @jdoerfert wrote: > The problem is this patch can only resolve to a single directive during > parsing. Take > > template > void foo() { > #pragma omp metadirective when(user={condition(b)}) ... >

[PATCH] D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location

protze.joachim added a comment. Reverting this commit fixes the compiler errors. This issue is tracked in https://bugs.llvm.org/show_bug.cgi?id=48953 , which is marked as blocking the 12 release. Without any knowledge about the compiler backend or the Debug codegen: might it be possible, that

[PATCH] D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location

protze.joachim added subscribers: jdoerfert, protze.joachim. protze.joachim added a comment. @jdoerfert This commit introduced the compile errors when OpenMP code is compiled with debug information. The symptom in llvm tests are the failing libarcher tests (which seem to be the only OpenMP tes

[PATCH] D94745: [OpenMP][deviceRTLs] Build the deviceRTLs with OpenMP instead of target dependent language

protze.joachim added a comment. Sorry, I meant to disable building of the cuda device-runtime, or whatever is breaking. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94745/new/ https://reviews.llvm.org/D94745 _

[PATCH] D94745: [OpenMP][deviceRTLs] Build the deviceRTLs with OpenMP instead of target dependent language

protze.joachim added a comment. In D94745#2539454 , @JonChesterfield wrote: > I think there's a bug report about this. Sycl (iirc) introduced a change that > caught invalid things with types that were previously ignored. @jdoerfert is > on point I think

[PATCH] D94745: [OpenMP][deviceRTLs] Build the deviceRTLs with OpenMP instead of target dependent language

protze.joachim added a comment. For me this patch breaks building llvm. Before this patch, I successfully built llvm using a llvm/10 installation on the system. What is probably special with our llvm installation is that we by default use libc++ rather than libstdc++. FAILED: projects/openmp

[PATCH] D91944: OpenMP 5.0 metadirective

protze.joachim added a comment. I found two issues with this patch regarding the `default` clause: - The spec does not require a default clause. I get `error: expected expression` if I omit a default clause. The error is gone if I add `default()`. - The spec does not allow an empty `default()` c

[PATCH] D65880: [Driver] Move LIBRARY_PATH before user inputs

protze.joachim added a comment. I still see some clang-specific and system link directories listed in the linker line before the directories from `LIBRARY_PATH`: $ LIBRARY_PATH=test1 /usr/local/clang/bin/clang -Ltest2 -v hello.c "/usr/bin/ld" .../crtbegin.o -Ltest2 -L/usr/lib/gcc/x86_64-re

[PATCH] D85934: Enable OpenMP offloading to VE and enable tests for offloading to VE

protze.joachim added inline comments. Comment at: openmp/libomptarget/test/lit.cfg:67 append_dynamic_library_path('DYLD_LIBRARY_PATH', \ config.omp_host_rtl_directory, ";") config.test_flags += " -Wl,-rpath," + config.library_dir Unrelated to t

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

protze.joachim reopened this revision. protze.joachim added a comment. This revision is now accepted and ready to land. I carefully made sure, that the freshly built clang was used to execute the test. I opened https://bugs.llvm.org/show_bug.cgi?id=46836 to track the issue and made it release bl

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

protze.joachim added a comment. This patch breaks compilation of previously working code. I added the following to `openmp/libomptarget/test/offloading/offloading_success.c`: +// RUN: %libomptarget-compile-run-and-check-nvptx64-nvidia-cuda which results in # command stderr: ptxas offloa

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

protze.joachim added a comment. I'm executing: $ /usr/bin/python bin/llvm-lit -vv -a projects/openmp/libomptarget/test/offloading/target_depend_nowait.cpp which executes: "/dev/shm/jprotze/build-tsan-fiber-as/./bin/clang++" "-fopenmp" "-pthread" "-fno-experimental-isel" "-I" "/dev/shm/jp

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

protze.joachim added a comment. Starting with the commit of this patch, the libomptarget test `llvm-project/openmp/libomptarget/test/offloading/target_depend_nowait.cpp` fails. Here is a stacktrace of the segfault: $ gdb target_depend_nowait.cpp.tmp-x86_64-pc-linux-gnu (gdb) run ... Pro

[PATCH] D64375: [OpenMP][Docs] Provide implementation status details

protze.joachim added inline comments. Comment at: clang/docs/OpenMPSupport.rst:165 ++--+--+--++ +| OMPD | OMPD i