Skip emitting duplicate clauses for CoreRT (#8400)

Fixes dotnet/corert#2262

Documentation/botr/clr-abi.md

@@ -472,7 +472,7 @@ Filters are invoked in the 1st pass of EH processing and as such execution might
 
 Duplicated clauses are a special set of entries in the EH tables to assist the VM. Specifically, if handler 'A' is also protected by an outer EH clause 'B', then the JIT must emit a duplicated clause, a duplicate of 'B', that marks the whole handler 'A' (which is now lexically disjoint for the range of code for the corresponding try body 'A') as being protected by the handler for 'B'.
 
-Duplicated clauses are not needed for x86.
+Duplicated clauses are not needed for x86 and for CoreRT ABI.
 
 During exception dispatch the VM uses these duplicated clauses to know when to skip any frames between the handler and its parent function. After skipping to the parent function, due to a duplicated clause, the VM searches for a regular/non-duplicate clause in the parent function. The order of duplicated clauses is important. They should appear after all of the main function clauses. They should still follow the normal sorting rules (inner-to-outer, top-to-bottom), but because the try-start/try-end will all be the same for a given handler, they should maintain the ordering, regarding inner-to-outer, as the corresponding original clause.
 

src/jit/codegencommon.cpp

@@ -3326,46 +3326,55 @@ void CodeGen::genReportEH()
     // VM.
     unsigned duplicateClauseCount = 0;
     unsigned enclosingTryIndex;
-    for (XTnum = 0; XTnum < compiler->compHndBBtabCount; XTnum++)
+
+    // Duplicate clauses are not used by CoreRT ABI
+    if (!compiler->IsTargetAbi(CORINFO_CORERT_ABI))
     {
-        for (enclosingTryIndex = compiler->ehTrueEnclosingTryIndexIL(XTnum); // find the true enclosing try index,
-                                                                             // ignoring 'mutual protect' trys
-             enclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX;
-             enclosingTryIndex = compiler->ehGetEnclosingTryIndex(enclosingTryIndex))
+        for (XTnum = 0; XTnum < compiler->compHndBBtabCount; XTnum++)
         {
-            ++duplicateClauseCount;
+            for (enclosingTryIndex = compiler->ehTrueEnclosingTryIndexIL(XTnum); // find the true enclosing try index,
+                                                                                 // ignoring 'mutual protect' trys
+                 enclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX;
+                 enclosingTryIndex = compiler->ehGetEnclosingTryIndex(enclosingTryIndex))
+            {
+                ++duplicateClauseCount;
+            }
         }
+        EHCount += duplicateClauseCount;
     }
-    EHCount += duplicateClauseCount;
 
 #if FEATURE_EH_CALLFINALLY_THUNKS
     unsigned clonedFinallyCount = 0;
 
-    // We don't keep track of how many cloned finally there are. So, go through and count.
-    // We do a quick pass first through the EH table to see if there are any try/finally
-    // clauses. If there aren't, we don't need to look for BBJ_CALLFINALLY.
-
-    bool anyFinallys = false;
-    for (HBtab = compiler->compHndBBtab, HBtabEnd = compiler->compHndBBtab + compiler->compHndBBtabCount;
-         HBtab < HBtabEnd; HBtab++)
+    // Duplicate clauses are not used by CoreRT ABI
+    if (!compiler->IsTargetAbi(CORINFO_CORERT_ABI))
     {
-        if (HBtab->HasFinallyHandler())
+        // We don't keep track of how many cloned finally there are. So, go through and count.
+        // We do a quick pass first through the EH table to see if there are any try/finally
+        // clauses. If there aren't, we don't need to look for BBJ_CALLFINALLY.
+
+        bool anyFinallys = false;
+        for (HBtab = compiler->compHndBBtab, HBtabEnd = compiler->compHndBBtab + compiler->compHndBBtabCount;
+             HBtab < HBtabEnd; HBtab++)
         {
-            anyFinallys = true;
-            break;
+            if (HBtab->HasFinallyHandler())
+            {
+                anyFinallys = true;
+                break;
+            }
         }
-    }
-    if (anyFinallys)
-    {
-        for (BasicBlock* block = compiler->fgFirstBB; block != nullptr; block = block->bbNext)
+        if (anyFinallys)
         {
-            if (block->bbJumpKind == BBJ_CALLFINALLY)
+            for (BasicBlock* block = compiler->fgFirstBB; block != nullptr; block = block->bbNext)
             {
-                ++clonedFinallyCount;
+                if (block->bbJumpKind == BBJ_CALLFINALLY)
+                {
+                    ++clonedFinallyCount;
+                }
             }
-        }
 
-        EHCount += clonedFinallyCount;
+            EHCount += clonedFinallyCount;
+        }
     }
 #endif // FEATURE_EH_CALLFINALLY_THUNKS
 
@@ -3664,7 +3673,7 @@ void CodeGen::genReportEH()
     } // if (duplicateClauseCount > 0)
 
 #if FEATURE_EH_CALLFINALLY_THUNKS
-    if (anyFinallys)
+    if (clonedFinallyCount > 0)
     {
         unsigned reportedClonedFinallyCount = 0;
         for (BasicBlock* block = compiler->fgFirstBB; block != nullptr; block = block->bbNext)
@@ -3718,7 +3727,7 @@ void CodeGen::genReportEH()
         }     // for each block
 
         assert(clonedFinallyCount == reportedClonedFinallyCount);
-    }  // if (anyFinallys)
+    }  // if (clonedFinallyCount > 0)
 #endif // FEATURE_EH_CALLFINALLY_THUNKS
 
 #endif // FEATURE_EH_FUNCLETS

src/jit/compiler.h

@@ -6383,10 +6383,19 @@ class Compiler
 #endif
     }
 
+    inline bool IsTargetAbi(CORINFO_RUNTIME_ABI abi)
+    {
+#if COR_JIT_EE_VERSION > 460
+        return eeGetEEInfo()->targetAbi == abi;
+#else
+        return CORINFO_DESKTOP_ABI == abi;
+#endif
+    }
+
     inline bool generateCFIUnwindCodes()
     {
-#if COR_JIT_EE_VERSION > 460 && defined(UNIX_AMD64_ABI)
-        return eeGetEEInfo()->targetAbi == CORINFO_CORERT_ABI;
+#ifdef UNIX_AMD64_ABI
+        return IsTargetAbi(CORINFO_CORERT_ABI);
 #else
         return false;
 #endif

src/jit/importer.cpp

@@ -5192,7 +5192,7 @@ void Compiler::impImportNewObjArray(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORI
     CLANG_FORMAT_COMMENT_ANCHOR;
 
 #if COR_JIT_EE_VERSION > 460
-    if (!opts.IsReadyToRun() || (eeGetEEInfo()->targetAbi == CORINFO_CORERT_ABI))
+    if (!opts.IsReadyToRun() || IsTargetAbi(CORINFO_CORERT_ABI))
     {
         LclVarDsc* newObjArrayArgsVar;
 
@@ -12383,14 +12383,12 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                 // At present this can only be String
                 else if (clsFlags & CORINFO_FLG_VAROBJSIZE)
                 {
-#if COR_JIT_EE_VERSION > 460
-                    if (eeGetEEInfo()->targetAbi == CORINFO_CORERT_ABI)
+                    if (IsTargetAbi(CORINFO_CORERT_ABI))
                     {
                         // The dummy argument does not exist in CoreRT
                         newObjThisPtr = nullptr;
                     }
                     else
-#endif
                     {
                         // This is the case for variable-sized objects that are not
                         // arrays.  In this case, call the constructor with a null 'this'

src/jit/lclvars.cpp

@@ -3388,10 +3388,8 @@ void Compiler::lvaMarkLocalVars()
 
 #endif // !FEATURE_EH_FUNCLETS
 
-#if COR_JIT_EE_VERSION > 460
     // PSPSym and LocAllocSPvar are not used by the CoreRT ABI
-    if (eeGetEEInfo()->targetAbi != CORINFO_CORERT_ABI)
-#endif
+    if (!IsTargetAbi(CORINFO_CORERT_ABI))
     {
 #if FEATURE_EH_FUNCLETS
         if (ehNeedsPSPSym())

src/jit/morph.cpp

@@ -16164,7 +16164,7 @@ GenTreePtr Compiler::fgInitThisClass()
     {
 #ifdef FEATURE_READYTORUN_COMPILER
         // Only CoreRT understands CORINFO_HELP_READYTORUN_GENERIC_STATIC_BASE. Don't do this on CoreCLR.
-        if (opts.IsReadyToRun() && eeGetEEInfo()->targetAbi == CORINFO_CORERT_ABI)
+        if (opts.IsReadyToRun() && IsTargetAbi(CORINFO_CORERT_ABI))
         {
             CORINFO_RESOLVED_TOKEN resolvedToken;
             memset(&resolvedToken, 0, sizeof(resolvedToken));