[OpenMP] Use `abort` not `error` for fatal runtime exceptions

[jdoerfert]

Summary

See PR46515 for the rational but generally, we want to *really* abort
not gracefully shut down.

[ABataev]

LG

[ye-luo]

Are you sure abort gets fprintf flushed?

[grokos]

abort() does not flush stdout, but here we are printing to stderr which is not buffered so there is no need to flush.

[ye-luo]

In D83963#2156495, @grokos wrote:

abort() does not flush stdout, but here we are printing to stderr which is not buffered so there is no need to flush.

Clear. Then everything is good. Let us have this on both master and 11.x

[jdoerfert]

Add flush

[ye-luo]

In D83963#2156786, @jdoerfert wrote:

Add flush

You missed the conversation above. stderr is not buffered and flush is not needed.

[jdoerfert]

In D83963#2156826, @ye-luo wrote:

In D83963#2156786, @jdoerfert wrote:

Add flush

You missed the conversation above. stderr is not buffered and flush is not needed.

I did, will remove it again..

[protze.joachim]

stderr is unbuffered by default. Are we sure this behavior is not changed when the output is redirected into a file, as for example during execution in a batch job? What were the worries about having the fflush there?

[jdoerfert]

In D83963#2174443, @protze.joachim wrote:

stderr is unbuffered by default. Are we sure this behavior is not changed when the output is redirected into a file, as for example during execution in a batch job? What were the worries about having the fflush there?

I went with this: https://reviews.llvm.org/D83963#2156612
If you think flush is needed, feel free to add it, it doesn't cost us anything.

[jdenny]

This change broke a number of libomptarget tests. Adding --crash to the %libomptarget-run-fail-* definitions in openmp/libomptarget/test/lit.cfg fixes that for me. Any objections?

[grokos]

Any idea why this change broke some tests? And how does --crash fix the problem?

[jdenny]

not expects a non-zero exit not a signal. Adding --crash adjusts the expectation.

I would've just pushed the fix, but I was concerned that maybe others weren't seeing this somehow.

[grokos]

Thanks, it makes sense. Can you submit a patch?

[jdenny]

I've pushed it to master as 9b4826d18b5f.

openmp/libomptarget/src/private.h

 
 #ifndef _OMPTARGET_PRIVATE_H
 #define _OMPTARGET_PRIVATE_H
 
 #include <omptarget.h>
 
 #include <cstdint>
 
 extern int target_data_begin(DeviceTy &Device, int32_t arg_num,
                              void **args_base, void **args, int64_t *arg_sizes,
                              int64_t *arg_types,
                              __tgt_async_info *async_info_ptr);
 
 extern int target_data_end(DeviceTy &Device, int32_t arg_num, void **args_base,
                            void **args, int64_t *arg_sizes, int64_t *arg_types,
                            __tgt_async_info *async_info_ptr);
 
 extern int target_data_update(DeviceTy &Device, int32_t arg_num,
     void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types);
 
 extern int target(int64_t device_id, void *host_ptr, int32_t arg_num,
     void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types,
     int32_t team_num, int32_t thread_limit, int IsTeamConstruct);
 
 extern int CheckDeviceAndCtors(int64_t device_id);
 
 // enum for OMP_TARGET_OFFLOAD; keep in sync with kmp.h definition

 // components are dynamically decided, so we utilize C++ STL vector
 // implementation here.
 struct MapperComponentsTy {
   std::vector<MapComponentInfoTy> Components;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 // implementation for fatal messages
 ////////////////////////////////////////////////////////////////////////////////
 
-#define FATAL_MESSAGE0(_num, _str)                                    \
-  do {                                                                \
-    fprintf(stderr, "Libomptarget fatal error %d: %s\n", _num, _str); \
-    exit(1);                                                          \
+#define FATAL_MESSAGE0(_num, _str)                                             \
+  do {                                                                         \
+    fprintf(stderr, "Libomptarget fatal error %d: %s\n", _num, _str);          \
+    fflush(stderr);                                                            \
+    abort();                                                                   \
   } while (0)
 
-#define FATAL_MESSAGE(_num, _str, ...)                              \
-  do {                                                              \
-    fprintf(stderr, "Libomptarget fatal error %d:" _str "\n", _num, \
-            __VA_ARGS__);                                           \
-    exit(1);                                                        \
+#define FATAL_MESSAGE(_num, _str, ...)                                         \
+  do {                                                                         \
+    fprintf(stderr, "Libomptarget fatal error %d:" _str "\n", _num,            \
+            __VA_ARGS__);                                                      \
+    fflush(stderr);                                                            \
+    abort();                                                                   \
   } while (0)
 
 // Implemented in libomp, they are called from within __tgt_* functions.
 #ifdef __cplusplus
 extern "C" {
 #endif
 // functions that extract info from libomp; keep in sync
 int omp_get_default_device(void) __attribute__((weak));