Reverts llvm/llvm-project#167879 This PR is causing assertions in the check-offload tests: https://lab.llvm.org/staging/#/builders/105 https://lab.llvm.org/staging/#/builders/105/builds/37057
Firstprivate pointers in OpenMP target regions were not being lowered correctly, causing the runtime to perform unnecessary present table lookups instead of passing pointer values directly. This patch adds the OMP_MAP_LITERAL flag for firstprivate pointers, enabling the runtime to pass pointer values directly without lookups. The fix handles both explicit firstprivate clauses and implicit firstprivate semantics from defaultmap clauses. Key changes: - Track defaultmap(firstprivate:...) clauses in MappableExprsHandler - Add isEffectivelyFirstprivate() to check both explicit and implicit firstprivate semantics - Apply OMP_MAP_LITERAL flag to firstprivate pointers in generateDefaultMapInfo() Map type values: - 288 = OMP_MAP_TARGET_PARAM | OMP_MAP_LITERAL (explicit firstprivate) - 800 = OMP_MAP_TARGET_PARAM | OMP_MAP_LITERAL | OMP_MAP_IS_PTR (implicit firstprivate from defaultmap) Before: Pointers got 544 (TARGET_PARAM | IS_PTR) causing runtime lookups After: Pointers get 288 or 800 (includes LITERAL) for direct pass Updated the 16 existing test cases in OpenMP that were expecting the previous (buggy) behavior. The tests were checking for map type values of 544 (TARGET_PARAM | IS_PTR) and 32 (TARGET_PARAM) for firstprivate pointers, which lacked the LITERAL flag (256). With this fix, firstprivate pointers now correctly include the LITERAL flag, resulting in map types 800 (TARGET_PARAM | LITERAL | IS_PTR) for implicit firstprivate and 288 (TARGET_PARAM | LITERAL) for explicit firstprivate. The updated tests now validate the correct behavior as per OpenMP 5.2 semantics, where firstprivate variables should be passed by value rather than requiring runtime present table lookups. --------- Co-authored-by: Sairudra More <moresair@pe31.hpc.amslabs.hpecorp.net> Co-authored-by: Alexey Bataev <a.bataev@gmx.com>
Summary: The previous offloading entry type did not fit the current use-cases very well. This widens it and adds a version to prevent further annoyances. It also includes the kind to better sort who's using it. The first 64-bytes are reserved as zero so the OpenMP runtime can detect the old format for binary compatibilitry.
Generate nuw GEPs for struct member accesses, as inbounds + non-negative implies nuw. Regression tests are updated using update scripts where possible, and by find + replace where not.
Summary: Currently, OpenMP handles the `omp requires` clause by emitting a global constructor into the runtime for every translation unit that requires it. However, this is not a great solution because it prevents us from having a defined order in which the runtime is accessed and used. This patch changes the approach to no longer use global constructors, but to instead group the flag with the other offloading entires that we already handle. This has the effect of still registering each flag per requires TU, but now we have a single constructor that handles everything. This function removes support for the old `__tgt_register_requires` and replaces it with a warning message. We just had a recent release, and the OpenMP policy for the past four releases since we switched to LLVM is that we do not provide strict backwards compatibility between major LLVM releases now that the library is versioned. This means that a user will need to recompile if they have an old binary that relied on `register_requires` having the old behavior. It is important that we actively deprecate this, as otherwise it would not solve the problem of having no defined init and shutdown order for `libomptarget`. The problem of `libomptarget` not having a define init and shutdown order cascades into a lot of other issues so I have a strong incentive to be rid of it. It is worth noting that the current `__tgt_offload_entry` only has space for a 32-bit integer here. I am planning to overhaul these at some point as well.
Reapplication of 7339c0f782d5c70e0928f8991b0c05338a90c84c with a fix for a crash involving arrays without a size expression. Clang supports VLAs in C++ as an extension, but we currently only warn on their use when you pass -Wvla, -Wvla-extension, or -pedantic. However, VLAs as they're expressed in C have been considered by WG21 and rejected, are easy to use accidentally to the surprise of users (e.g., https://ddanilov.me/default-non-standard-features/), and they have potential security implications beyond constant-size arrays (https://wiki.sei.cmu.edu/confluence/display/c/ARR32-C.+Ensure+size+arguments+for+variable+length+arrays+are+in+a+valid+range). C++ users should strongly consider using other functionality such as std::vector instead. This seems like sufficiently compelling evidence to warn users about VLA use by default in C++ modes. This patch enables the -Wvla-extension diagnostic group in C++ language modes by default, and adds the warning group to -Wall in GNU++ language modes. The warning is still opt-in in C language modes, where support for VLAs is somewhat less surprising to users. RFC: https://discourse.llvm.org/t/rfc-diagnosing-use-of-vlas-in-c/73109 Fixes https://github.com/llvm/llvm-project/issues/62836 Differential Revision: https://reviews.llvm.org/D156565
This reverts commit 7339c0f782d5c70e0928f8991b0c05338a90c84c. Breaks bots: https://lab.llvm.org/buildbot/#/builders/139/builds/51875 https://lab.llvm.org/buildbot/#/builders/164/builds/45262
Clang supports VLAs in C++ as an extension, but we currently only warn on their use when you pass -Wvla, -Wvla-extension, or -pedantic. However, VLAs as they're expressed in C have been considered by WG21 and rejected, are easy to use accidentally to the surprise of users (e.g., https://ddanilov.me/default-non-standard-features/), and they have potential security implications beyond constant-size arrays (https://wiki.sei.cmu.edu/confluence/display/c/ARR32-C.+Ensure+size+arguments+for+variable+length+arrays+are+in+a+valid+range). C++ users should strongly consider using other functionality such as std::vector instead. This seems like sufficiently compelling evidence to warn users about VLA use by default in C++ modes. This patch enables the -Wvla-extension diagnostic group in C++ language modes by default, and adds the warning group to -Wall in GNU++ language modes. The warning is still opt-in in C language modes, where support for VLAs is somewhat less surprising to users. RFC: https://discourse.llvm.org/t/rfc-diagnosing-use-of-vlas-in-c/73109 Fixes https://github.com/llvm/llvm-project/issues/62836 Differential Revision: https://reviews.llvm.org/D156565
This patch renames the `OpenMPIRBuilderConfig` flags to reduce confusion over their meaning. `IsTargetCodegen` becomes `IsGPU`, whereas `IsEmbedded` becomes `IsTargetDevice`. The `-fopenmp-is-device` compiler option is also renamed to `-fopenmp-is-target-device` and the `omp.is_device` MLIR attribute is renamed to `omp.is_target_device`. Getters and setters of all these renamed properties are also updated accordingly. Many unit tests have been updated to use the new names, but an alias for the `-fopenmp-is-device` option is created so that external programs do not stop working after the name change. `IsGPU` is set when the target triple is AMDGCN or NVIDIA PTX, and it is only valid if `IsTargetDevice` is specified as well. `IsTargetDevice` is set by the `-fopenmp-is-target-device` compiler frontend option, which is only added to the OpenMP device invocation for offloading-enabled programs. Differential Revision: https://reviews.llvm.org/D154591
Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D150682