[flang-commits] [PATCH] D126291: [flang][Driver] Update link job on windows

[Martin Storsjö via Phabricator via flang-commits]

mstorsjo added a comment.

In D126291#3556324 <https://reviews.llvm.org/D126291#3556324>, @mmuetzel wrote:

> With this additional change, I no longer need the `-lc++` flag:
>
>   From 965343d8b05bf3cf7a9a3873ea4d2ddcc00a3703 Mon Sep 17 00:00:00 2001
>   From: =?UTF-8?q?Markus=20M=C3=BCtzel?= <markus.muetzel at gmx.de>
>   Date: Fri, 3 Jun 2022 19:27:27 +0200
>   Subject: [PATCH] lock without <mutex> on Windows
>   
>   ---
>    flang/runtime/lock.h | 24 ++++++++++--------------
>    1 file changed, 10 insertions(+), 14 deletions(-)
>   
>   diff --git a/flang/runtime/lock.h b/flang/runtime/lock.h
>   index 0abc1158c2c1..b088bd6bb8e1 100644
>   --- a/flang/runtime/lock.h
>   +++ b/flang/runtime/lock.h
>   @@ -15,23 +15,17 @@
>    
>    // Avoid <mutex> if possible to avoid introduction of C++ runtime
>    // library dependence.
>   -#ifndef _WIN32
>   -#define USE_PTHREADS 1
>   +#ifdef _WIN32
>   +#include <windows.h>
>    #else
>   -#undef USE_PTHREADS
>   -#endif
>   -
>   -#if USE_PTHREADS
>    #include <pthread.h>
>   -#else
>   -#include <mutex>
>    #endif
>    
>    namespace Fortran::runtime {
>    
>    class Lock {
>    public:
>   -#if USE_PTHREADS
>   +#ifndef _WIN32
>      Lock() { pthread_mutex_init(&mutex_, nullptr); }
>      ~Lock() { pthread_mutex_destroy(&mutex_); }
>      void Take() {
>   @@ -41,9 +35,11 @@ public:
>      bool Try() { return pthread_mutex_trylock(&mutex_) == 0; }
>      void Drop() { pthread_mutex_unlock(&mutex_); }
>    #else
>   -  void Take() { mutex_.lock(); }
>   -  bool Try() { return mutex_.try_lock(); }
>   -  void Drop() { mutex_.unlock(); }
>   +  Lock() { mutex_=CreateMutex(nullptr, FALSE, nullptr); }
>   +  ~Lock() { CloseHandle(mutex_); }
>   +  void Take() { WaitForSingleObject(mutex_, INFINITE); }
>   +  bool Try() { return WaitForSingleObject(mutex_, 0) == 0; }
>   +  void Drop() { ReleaseMutex(mutex_); }
>    #endif
>    
>      void CheckLocked(const Terminator &terminator) {
>   @@ -54,10 +50,10 @@ public:
>      }
>    
>    private:
>   -#if USE_PTHREADS
>   +#ifndef _WIN32
>      pthread_mutex_t mutex_{};
>    #else
>   -  std::mutex mutex_;
>   +  HANDLE mutex_;
>    #endif
>    };
>    
>   -- 
>   2.35.3.windows.1
>
> The previous logic was: Use <mutex> on Windows and pthread everywhere else. The new logic is: Use the WinAPI (CreateMutex and Co) on Windows and pthread everywhere else.
> Someone should check if that change is sane. But it seems to work for me (with the "Hello World" example).

This looks mostly reasonable, but I'd recommend using a windows critical section instead of a mutex - a critical section doesn't invoke the kernel when there's no contention of the lock. For that, you'd use `CRITICAL_SECTION`, `InitializeCriticalSection(&cs);`, `EnterCriticalSection(&cs);`, `LeaveCriticalSection(&cs);`, `DeleteCriticalSection(&cs);`.


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

https://reviews.llvm.org/D126291