Support for synchronized methods.

Add calls to MONITOR_ENTER on entry to the method and MONITOR_EXIT on returns.

We can't catch exceptions yet; when we can we will need to add a try/fault for the entire method
so that we can call MONITOR_EXIT on unhandled exceptions.

We have a synchronized method in HelloWorld (System.IO.TextWriter+SyncTextWriter.WriteLine).
I verified that the generated IR looks correct. I also tested with a simple two-thread race that the
behavior of synchronized instance and static methods is as expected.

include/Reader/reader.h

@@ -2839,7 +2839,7 @@ public:
   virtual IRNode *refAnyType(IRNode *Arg1) = 0;
   virtual IRNode *refAnyVal(IRNode *Val, CORINFO_RESOLVED_TOKEN *ResolvedToken);
   virtual void rethrow() = 0;
-  virtual void returnOpcode(IRNode *Opr, bool IsSynchronousMethod) = 0;
+  virtual void returnOpcode(IRNode *Opr, bool IsSynchronizedMethod) = 0;
   virtual IRNode *shift(ReaderBaseNS::ShiftOpcode Opcode, IRNode *ShiftAmount,
                         IRNode *ShiftOperand) = 0;
   virtual IRNode *sizeofOpcode(CORINFO_RESOLVED_TOKEN *ResolvedToken) = 0;

include/Reader/readerir.h

@@ -436,7 +436,7 @@ public:
   };
 
   void rethrow() override { throw NotYetImplementedException("rethrow"); };
-  void returnOpcode(IRNode *Opr, bool SynchronousMethod) override;
+  void returnOpcode(IRNode *Opr, bool IsSynchronizedMethod) override;
   IRNode *shift(ReaderBaseNS::ShiftOpcode Opcode, IRNode *ShiftAmount,
                 IRNode *ShiftOperand) override;
   IRNode *sizeofOpcode(CORINFO_RESOLVED_TOKEN *ResolvedToken) override;
@@ -755,6 +755,13 @@ public:
   IRNode *callRuntimeHandleHelper(CorInfoHelpFunc Helper, IRNode *Arg1,
                                   IRNode *Arg2, IRNode *NullCheckArg);
 
+  /// Generate a helper call to enter or exit a monitor used by synchronized
+  /// methods.
+  ///
+  /// \param IsEnter true if the monitor should be entered; false if the monitor
+  /// should be exited.
+  void callMonitorHelper(bool IsEnter);
+
   IRNode *convertToBoxHelperArgumentType(IRNode *Opr,
                                          CorInfoType CorType) override;
 
@@ -1274,6 +1281,13 @@ private:
   bool NeedsSecurityObject;
   llvm::BasicBlock *EntryBlock;
   llvm::Instruction *TempInsertionPoint;
+  IRNode *MethodSyncHandle; ///< If the method is synchronized, this is
+                            ///< the handle used for entering and exiting
+                            ///< the monitor.
+  llvm::Value *SyncFlag;    ///< For synchronized methods this flag
+                            ///< indicates whether the monitor has been
+                            ///< entered. It is set and checked by monitor
+                            ///< helpers.
   uint32_t TargetPointerSizeInBits;
   const uint32_t UnmanagedAddressSpace = 0;
   const uint32_t ManagedAddressSpace = 1;

lib/Reader/readerir.cpp

@@ -303,9 +303,10 @@ void GenIR::readerPrePass(uint8_t *Buffer, uint32_t NumBytes) {
       ABIMethodSignature(MethodSignature, *this, *JitContext->TheABIInfo);
   Function = ABIMethodSig.createFunction(*this, *JitContext->CurrentModule);
 
-  EntryBlock = BasicBlock::Create(*JitContext->LLVMContext, "entry", Function);
+  llvm::LLVMContext &LLVMContext = *JitContext->LLVMContext;
+  EntryBlock = BasicBlock::Create(LLVMContext, "entry", Function);
 
-  LLVMBuilder = new IRBuilder<>(*this->JitContext->LLVMContext);
+  LLVMBuilder = new IRBuilder<>(LLVMContext);
   LLVMBuilder->SetInsertPoint(EntryBlock);
 
   // Note numArgs may exceed the IL argument count when there
@@ -367,9 +368,26 @@ void GenIR::readerPrePass(uint8_t *Buffer, uint32_t NumBytes) {
   // Check for special cases where the Jit needs to do extra work.
   const uint32_t MethodFlags = getCurrentMethodAttribs();
 
-  // TODO: support for synchronized methods
+  // Check for synchronized method. If a method is synchronized the JIT is
+  // required to insert a helper call to MONITOR_ENTER before we start the user
+  // code. A similar call to MONITOR_EXIT will be placed at the return site.
+  // TODO: when we start catching exceptions we should create a try/fault for
+  // the entire method so that we can exit the monitor on unhandled exceptions.
+  MethodSyncHandle = nullptr;
+  SyncFlag = nullptr;
   if (MethodFlags & CORINFO_FLG_SYNCH) {
-    throw NotYetImplementedException("synchronized method");
+    MethodSyncHandle = rdrGetCritSect();
+    Type *SyncFlagType = Type::getInt8Ty(LLVMContext);
+
+    // Create address taken local SyncFlag. This will be passed by address to
+    // MONITOR_ENTER and MONITOR_EXIT. MONITOR_ENTER will set SyncFlag once lock
+    // has been obtained, MONITOR_EXIT only releases lock if SyncFlag is set.
+    SyncFlag = createTemporary(SyncFlagType, "SyncFlag");
+    Constant *ZeroConst = Constant::getNullValue(SyncFlagType);
+    LLVMBuilder->CreateStore(ZeroConst, SyncFlag);
+
+    const bool IsEnter = true;
+    callMonitorHelper(IsEnter);
   }
 
   if ((JitFlags & CORJIT_FLG_DEBUG_CODE) && !(JitFlags & CORJIT_FLG_IL_STUB)) {
@@ -539,6 +557,19 @@ void GenIR::insertIRForSecurityObject() {
   // via the GC encoding.
 }
 
+void GenIR::callMonitorHelper(bool IsEnter) {
+  CorInfoHelpFunc HelperId;
+  const uint32_t MethodFlags = getCurrentMethodAttribs();
+  if (MethodFlags & CORINFO_FLG_STATIC) {
+    HelperId =
+        IsEnter ? CORINFO_HELP_MON_ENTER_STATIC : CORINFO_HELP_MON_EXIT_STATIC;
+  } else {
+    HelperId = IsEnter ? CORINFO_HELP_MON_ENTER : CORINFO_HELP_MON_EXIT;
+  }
+  callHelperImpl(HelperId, Type::getVoidTy(*JitContext->LLVMContext),
+                 MethodSyncHandle, (IRNode *)SyncFlag);
+}
+
 #pragma endregion
 
 #pragma region UTILITIES
@@ -4236,44 +4267,56 @@ bool GenIR::fgOptRecurse(mdToken Token) {
   return true;
 }
 
-void GenIR::returnOpcode(IRNode *Opr, bool IsSynchronousMethod) {
+void GenIR::returnOpcode(IRNode *Opr, bool IsSynchronizedMethod) {
   const ABIArgInfo &ResultInfo = ABIMethodSig.getResultInfo();
   const CallArgType &ResultArgType = MethodSignature.getResultType();
   CorInfoType ResultCorType = ResultArgType.CorType;
+  Value *ResultValue = nullptr;
+  bool IsVoidReturn = ResultCorType == CORINFO_TYPE_VOID;
 
-  if (ResultCorType == CORINFO_TYPE_VOID) {
+  if (IsVoidReturn) {
     assert(Opr == nullptr);
     assert(ResultInfo.getType()->isVoidTy());
-    LLVMBuilder->CreateRetVoid();
-    return;
-  }
-
-  assert(Opr != nullptr);
-
-  Value *ResultValue = nullptr;
-  if (ResultInfo.getKind() == ABIArgInfo::Indirect) {
-    Type *ResultTy = ResultInfo.getType();
-    ResultValue = convertFromStackType(Opr, ResultCorType, ResultTy);
-
-    CORINFO_CLASS_HANDLE ResultClass = ResultArgType.Class;
-    if (JitContext->JitInfo->isStructRequiringStackAllocRetBuf(ResultClass) ==
-        TRUE) {
-      // The return buffer must be on the stack; a simple store will suffice.
-      LLVMBuilder->CreateStore(ResultValue, IndirectResult);
-    } else {
-      const bool IsVolatile = false;
-      storeIndirectArg(ResultArgType, ResultValue, IndirectResult, IsVolatile);
-    }
-
-    ResultValue = IndirectResult;
   } else {
-    Type *ResultTy = getType(ResultCorType, ResultArgType.Class);
-    ResultValue = convertFromStackType(Opr, ResultCorType, ResultTy);
-    ResultValue =
-        ABISignature::coerce(*this, ResultInfo.getType(), ResultValue);
+    assert(Opr != nullptr);
+
+    if (ResultInfo.getKind() == ABIArgInfo::Indirect) {
+      Type *ResultTy = ResultInfo.getType();
+      ResultValue = convertFromStackType(Opr, ResultCorType, ResultTy);
+
+      CORINFO_CLASS_HANDLE ResultClass = ResultArgType.Class;
+      if (JitContext->JitInfo->isStructRequiringStackAllocRetBuf(ResultClass) ==
+          TRUE) {
+        // The return buffer must be on the stack; a simple store will suffice.
+        LLVMBuilder->CreateStore(ResultValue, IndirectResult);
+      } else {
+        const bool IsVolatile = false;
+        storeIndirectArg(ResultArgType, ResultValue, IndirectResult,
+                         IsVolatile);
+      }
+
+      ResultValue = IndirectResult;
+    } else {
+      Type *ResultTy = getType(ResultCorType, ResultArgType.Class);
+      ResultValue = convertFromStackType(Opr, ResultCorType, ResultTy);
+      ResultValue =
+          ABISignature::coerce(*this, ResultInfo.getType(), ResultValue);
+    }
   }
 
-  LLVMBuilder->CreateRet(ResultValue);
+  // If the method is synchronized, then we must insert a call to MONITOR_EXIT
+  // before returning. The call to MONITOR_EXIT must occur after the return
+  // value has been calculated.
+  if (IsSynchronizedMethod) {
+    const bool IsEnter = false;
+    callMonitorHelper(IsEnter);
+  }
+
+  if (IsVoidReturn) {
+    LLVMBuilder->CreateRetVoid();
+  } else {
+    LLVMBuilder->CreateRet(ResultValue);
+  }
 }
 
 bool GenIR::needSequencePoints() { return false; }