[PATCH] D128907: [Clang] Disable noundef attribute for languages which allow uninitialized function arguments

Wed Jul 13 21:44:22 PDT 2022

sameerds added a comment.

The current set of reviewers is mostly loaded with HIP engineers who are familiar with the issue and the proposed solution. But this solution affects all languages with convergent functions, which is visible from the affected tests. It will be good to seek comments from people responsible for CUDA, OpenMP and OpenCL too.

================
Comment at: clang/include/clang/Basic/LangOptions.h:534
+  /// Return true if uninitialized function arguments are allowed.
+  bool allowUninitializedFunctionsArgs() const {
+    /// CUDA/HIP etc. cross-lane APIs are convergent functions
----------------
The spelling should be "allowUninitializedFunctionArgs()", where the noun "Function" is singular.

================
Comment at: clang/lib/CodeGen/CGCall.cpp:2306

+  // Enable noundef attribute based on codegen options and
+  // skip adding the attribute for languages which allows uninitialized
----------------
This comment is merely an English translation of the Boolean expression. It should instead provide a bit of context. Like the fact that on HIP, CUDA, etc, some functions have multi-threaded semantics where it is enough for only one or some threads to provide defined arguments. Depending on semantics, undef arguments in some threads don't produce undefined results in the function call.

It might be worth adding this longer explanation to the commit description too, rather than just saying "strict constraints".

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128907