Make JitDump label output multi-thread safe (#32578)
authorKunal Pathak <Kunal.Pathak@microsoft.com>
Thu, 27 Feb 2020 17:19:23 +0000 (09:19 -0800)
committerGitHub <noreply@github.com>
Thu, 27 Feb 2020 17:19:23 +0000 (09:19 -0800)
* Make JitDump label output multi-thread safe

`s_compMethodsCount` is a static variable and incrementing it when
two JITs are running at the same time can result in inconsistent labels. This PR fixes it by adding an instance variable `compMethodID` on `Compiler` object and setting it to the value of `s_compMethodsCount` after incrementing it using `InterlockedIncrement()`.

Changed the type of `s_compMethodID` to `LONG` to match the signature of `InterlockedIncrement()` and made it private.

Fixes #10984

13 files changed:
docs/design/coreclr/botr/ryujit-overview.md
docs/workflow/debugging/coreclr/debugging.md
src/coreclr/src/jit/codegenarm.cpp
src/coreclr/src/jit/codegenarm64.cpp
src/coreclr/src/jit/codegencommon.cpp
src/coreclr/src/jit/codegenxarch.cpp
src/coreclr/src/jit/compiler.cpp
src/coreclr/src/jit/compiler.h
src/coreclr/src/jit/emit.cpp
src/coreclr/src/jit/emit.h
src/coreclr/src/jit/emitarm.cpp
src/coreclr/src/jit/emitarm64.cpp
src/coreclr/src/jit/emitxarch.cpp

index b214467..b91054d 100644 (file)
@@ -765,11 +765,11 @@ you want to dump. For example:
 set "COMPlus_JitDump=Main GetEnumerator"
 ```
 
-See [Setting configuration variables](../../../workflow/testing/viewing-jit-dumps.md#setting-configuration-variables) for more
+See [Setting configuration variables](../../../workflow/testing/coreclr/viewing-jit-dumps.md#setting-configuration-variables) for more
 details on this.
 
 Full instructions for dumping the compilation of some managed code can be found here:
-[viewing-jit-dumps.md](../../../workflow/testing/viewing-jit-dumps.md)
+[viewing-jit-dumps.md](../../../workflow/testing/coreclr/viewing-jit-dumps.md)
 
 ## Reading expression trees
 
index 097c767..ce7ac44 100644 (file)
@@ -9,7 +9,7 @@ Debugging CoreCLR on Windows
 ============================
 
 1. Perform a build of the repo.
-2. Open solution \<reporoot\>\artifacts\obj\Windows_NT.\<platform\>.\<configuration\>\CoreCLR.sln in Visual Studio. \<platform\> and \<configuration\> are based
+2. Open solution \<reporoot\>\artifacts\obj\coreclr\Windows_NT.\<platform\>.\<configuration\>\CoreCLR.sln in Visual Studio. \<platform\> and \<configuration\> are based
     on type of build you did. By default they are 'x64' and 'Debug'.
 3. Right-click the INSTALL project and choose â€˜Set as StartUp Project’
 4. Bring up the properties page for the INSTALL project
index 3afd7e3..f4d7f92 100644 (file)
@@ -624,14 +624,14 @@ void CodeGen::genJumpTable(GenTree* treeNode)
 
     jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, false);
 
-    JITDUMP("\n      J_M%03u_DS%02u LABEL   DWORD\n", Compiler::s_compMethodsCount, jmpTabBase);
+    JITDUMP("\n      J_M%03u_DS%02u LABEL   DWORD\n", compiler->compMethodID, jmpTabBase);
 
     for (unsigned i = 0; i < jumpCount; i++)
     {
         BasicBlock* target = *jumpTable++;
         noway_assert(target->bbFlags & BBF_JMP_TARGET);
 
-        JITDUMP("            DD      L_M%03u_" FMT_BB "\n", Compiler::s_compMethodsCount, target->bbNum);
+        JITDUMP("            DD      L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum);
 
         GetEmitter()->emitDataGenData(i, target);
     }
index 7837fd9..da26c13 100644 (file)
@@ -2682,14 +2682,14 @@ void CodeGen::genJumpTable(GenTree* treeNode)
 
     jmpTabOffs = 0;
 
-    JITDUMP("\n      J_M%03u_DS%02u LABEL   DWORD\n", Compiler::s_compMethodsCount, jmpTabBase);
+    JITDUMP("\n      J_M%03u_DS%02u LABEL   DWORD\n", compiler->compMethodID, jmpTabBase);
 
     for (unsigned i = 0; i < jumpCount; i++)
     {
         BasicBlock* target = *jumpTable++;
         noway_assert(target->bbFlags & BBF_JMP_TARGET);
 
-        JITDUMP("            DD      L_M%03u_" FMT_BB "\n", Compiler::s_compMethodsCount, target->bbNum);
+        JITDUMP("            DD      L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum);
 
         GetEmitter()->emitDataGenData(i, target);
     };
index 91457cf..7da93af 100644 (file)
@@ -998,7 +998,7 @@ void CodeGen::genLogLabel(BasicBlock* bb)
 #ifdef DEBUG
     if (compiler->opts.dspCode)
     {
-        printf("\n      L_M%03u_" FMT_BB ":\n", Compiler::s_compMethodsCount, bb->bbNum);
+        printf("\n      L_M%03u_" FMT_BB ":\n", compiler->compMethodID, bb->bbNum);
     }
 #endif
 }
@@ -10474,7 +10474,7 @@ void CodeGen::genIPmappingDisp(unsigned mappingNum, Compiler::IPmappingDsc* ipMa
     }
 
     printf(" ");
-    ipMapping->ipmdNativeLoc.Print();
+    ipMapping->ipmdNativeLoc.Print(compiler->compMethodID);
     // We can only call this after code generation. Is there any way to tell when it's legal to call?
     // printf(" [%x]", ipMapping->ipmdNativeLoc.CodeOffset(GetEmitter()));
 
index d94d043..d001a26 100644 (file)
@@ -3808,14 +3808,14 @@ void CodeGen::genJumpTable(GenTree* treeNode)
 
     jmpTabOffs = 0;
 
-    JITDUMP("\n      J_M%03u_DS%02u LABEL   DWORD\n", Compiler::s_compMethodsCount, jmpTabBase);
+    JITDUMP("\n      J_M%03u_DS%02u LABEL   DWORD\n", compiler->compMethodID, jmpTabBase);
 
     for (unsigned i = 0; i < jumpCount; i++)
     {
         BasicBlock* target = *jumpTable++;
         noway_assert(target->bbFlags & BBF_JMP_TARGET);
 
-        JITDUMP("            DD      L_M%03u_" FMT_BB "\n", Compiler::s_compMethodsCount, target->bbNum);
+        JITDUMP("            DD      L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum);
 
         GetEmitter()->emitDataGenData(i, target);
     };
index ffac4e1..c6c0969 100644 (file)
@@ -1299,7 +1299,7 @@ size_t genFlowNodeCnt;
 
 #ifdef DEBUG
 /* static */
-unsigned Compiler::s_compMethodsCount = 0; // to produce unique label names
+LONG Compiler::s_compMethodsCount = 0; // to produce unique label names
 #endif
 
 #if MEASURE_MEM_ALLOC
@@ -5997,11 +5997,11 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr,
 
     if (opts.disAsm || opts.dspEmit || verbose)
     {
-        s_compMethodsCount = ~info.compMethodHash() & 0xffff;
+        compMethodID = ~info.compMethodHash() & 0xffff;
     }
     else
     {
-        s_compMethodsCount++;
+        compMethodID = InterlockedIncrement(&s_compMethodsCount);
     }
 #endif
 
index 6028877..30f59fe 100644 (file)
@@ -8992,10 +8992,16 @@ public:
 //-------------------------- Global Compiler Data ------------------------------------
 
 #ifdef DEBUG
-    static unsigned s_compMethodsCount; // to produce unique label names
-    unsigned        compGenTreeID;
-    unsigned        compStatementID;
-    unsigned        compBasicBlockID;
+private:
+    static LONG s_compMethodsCount; // to produce unique label names
+#endif
+
+public:
+#ifdef DEBUG
+    LONG     compMethodID;
+    unsigned compGenTreeID;
+    unsigned compStatementID;
+    unsigned compBasicBlockID;
 #endif
 
     BasicBlock* compCurBB;   // the current basic block in process
index 65e6886..3069502 100644 (file)
@@ -81,11 +81,11 @@ bool emitLocation::IsPreviousInsNum(const emitter* emit) const
 }
 
 #ifdef DEBUG
-void emitLocation::Print() const
+void emitLocation::Print(LONG compMethodID) const
 {
     unsigned insNum = emitGetInsNumFromCodePos(codePos);
     unsigned insOfs = emitGetInsOfsFromCodePos(codePos);
-    printf("(G_M%03u_IG%02u,ins#%d,ofs#%d)", Compiler::s_compMethodsCount, ig->igNum, insNum, insOfs);
+    printf("(G_M%03u_IG%02u,ins#%d,ofs#%d)", compMethodID, ig->igNum, insNum, insOfs);
 }
 #endif // DEBUG
 
@@ -824,7 +824,7 @@ insGroup* emitter::emitSavIG(bool emitAdd)
 #ifdef DEBUG
     if (emitComp->opts.dspCode)
     {
-        printf("\n      G_M%03u_IG%02u:", Compiler::s_compMethodsCount, ig->igNum);
+        printf("\n      G_M%03u_IG%02u:", emitComp->compMethodID, ig->igNum);
         if (emitComp->verbose)
         {
             printf("        ; offs=%06XH, funclet=%02u, bbWeight=%s", ig->igOffs, ig->igFuncIdx,
@@ -3243,7 +3243,7 @@ void emitter::emitDispIG(insGroup* ig, insGroup* igPrev, bool verbose)
     const int TEMP_BUFFER_LEN = 40;
     char      buff[TEMP_BUFFER_LEN];
 
-    sprintf_s(buff, TEMP_BUFFER_LEN, "G_M%03u_IG%02u:        ", Compiler::s_compMethodsCount, ig->igNum);
+    sprintf_s(buff, TEMP_BUFFER_LEN, "G_M%03u_IG%02u:        ", emitComp->compMethodID, ig->igNum);
     printf("%s; ", buff);
     if ((igPrev == nullptr) || (igPrev->igFuncIdx != ig->igFuncIdx))
     {
@@ -3873,7 +3873,7 @@ AGAIN:
             {
                 printf("Binding: ");
                 emitDispIns(jmp, false, false, false);
-                printf("Binding L_M%03u_" FMT_BB, Compiler::s_compMethodsCount, jmp->idAddr()->iiaBBlabel->bbNum);
+                printf("Binding L_M%03u_" FMT_BB, emitComp->compMethodID, jmp->idAddr()->iiaBBlabel->bbNum);
             }
 #endif // DEBUG
 
@@ -3884,7 +3884,7 @@ AGAIN:
             {
                 if (tgtIG)
                 {
-                    printf("to G_M%03u_IG%02u\n", Compiler::s_compMethodsCount, tgtIG->igNum);
+                    printf("to G_M%03u_IG%02u\n", emitComp->compMethodID, tgtIG->igNum);
                 }
                 else
                 {
@@ -4929,7 +4929,7 @@ unsigned emitter::emitEndCodeGen(Compiler* comp,
             }
             else
             {
-                printf("\nG_M%03u_IG%02u:\n", Compiler::s_compMethodsCount, ig->igNum);
+                printf("\nG_M%03u_IG%02u:\n", emitComp->compMethodID, ig->igNum);
             }
         }
 #endif // DEBUG
@@ -5773,9 +5773,8 @@ void emitter::emitDispDataSec(dataSecDsc* section)
                 const char* blockLabelFormat = "G_M%03u_IG%02u";
                 char        blockLabel[64];
                 char        firstLabel[64];
-                sprintf_s(blockLabel, _countof(blockLabel), blockLabelFormat, Compiler::s_compMethodsCount, ig->igNum);
-                sprintf_s(firstLabel, _countof(firstLabel), blockLabelFormat, Compiler::s_compMethodsCount,
-                          igFirst->igNum);
+                sprintf_s(blockLabel, _countof(blockLabel), blockLabelFormat, emitComp->compMethodID, ig->igNum);
+                sprintf_s(firstLabel, _countof(firstLabel), blockLabelFormat, emitComp->compMethodID, igFirst->igNum);
 
                 if (isRelative)
                 {
@@ -7591,7 +7590,7 @@ const char* emitter::emitOffsetToLabel(unsigned offs)
         if (ig->igOffs == offs)
         {
             // Found it!
-            sprintf_s(buf[curBuf], TEMP_BUFFER_LEN, "G_M%03u_IG%02u", Compiler::s_compMethodsCount, ig->igNum);
+            sprintf_s(buf[curBuf], TEMP_BUFFER_LEN, "G_M%03u_IG%02u", emitComp->compMethodID, ig->igNum);
             retbuf = buf[curBuf];
             curBuf = (curBuf + 1) % 4;
             return retbuf;
index 622d6d6..2a3e5a7 100644 (file)
@@ -194,7 +194,7 @@ public:
     bool emitLocation::IsPreviousInsNum(const emitter* emit) const;
 
 #ifdef DEBUG
-    void Print() const;
+    void Print(LONG compMethodID) const;
 #endif // DEBUG
 
 private:
index 8c60e0f..b5ed5e8 100644 (file)
@@ -7072,8 +7072,8 @@ void emitter::emitDispInsHelp(
                 {
                     printf("reloc ");
                 }
-                printf("%s ADDRESS J_M%03u_DS%02u", (id->idIns() == INS_movw) ? "LOW" : "HIGH",
-                       Compiler::s_compMethodsCount, imm);
+                printf("%s ADDRESS J_M%03u_DS%02u", (id->idIns() == INS_movw) ? "LOW" : "HIGH", emitComp->compMethodID,
+                       imm);
 
                 // After the MOVT, dump the table
                 if (id->idIns() == INS_movt)
@@ -7085,7 +7085,7 @@ void emitter::emitDispInsHelp(
 
                     if (isBound)
                     {
-                        printf("\n\n    J_M%03u_DS%02u LABEL   DWORD", Compiler::s_compMethodsCount, imm);
+                        printf("\n\n    J_M%03u_DS%02u LABEL   DWORD", emitComp->compMethodID, imm);
 
                         /* Display the label table (it's stored as "BasicBlock*" values) */
 
@@ -7098,7 +7098,7 @@ void emitter::emitDispInsHelp(
                             lab = (insGroup*)emitCodeGetCookie(*bbp++);
                             assert(lab);
 
-                            printf("\n            DD      G_M%03u_IG%02u", Compiler::s_compMethodsCount, lab->igNum);
+                            printf("\n            DD      G_M%03u_IG%02u", emitComp->compMethodID, lab->igNum);
                         } while (--cnt);
                     }
                 }
@@ -7407,9 +7407,9 @@ void emitter::emitDispInsHelp(
         case IF_T2_M1: // Load Label
             emitDispReg(id->idReg1(), attr, true);
             if (id->idIsBound())
-                printf("G_M%03u_IG%02u", Compiler::s_compMethodsCount, id->idAddr()->iiaIGlabel->igNum);
+                printf("G_M%03u_IG%02u", emitComp->compMethodID, id->idAddr()->iiaIGlabel->igNum);
             else
-                printf("L_M%03u_" FMT_BB, Compiler::s_compMethodsCount, id->idAddr()->iiaBBlabel->bbNum);
+                printf("L_M%03u_" FMT_BB, emitComp->compMethodID, id->idAddr()->iiaBBlabel->bbNum);
             break;
 
         case IF_T1_I: // Special Compare-and-branch
@@ -7452,9 +7452,9 @@ void emitter::emitDispInsHelp(
                 }
             }
             else if (id->idIsBound())
-                printf("G_M%03u_IG%02u", Compiler::s_compMethodsCount, id->idAddr()->iiaIGlabel->igNum);
+                printf("G_M%03u_IG%02u", emitComp->compMethodID, id->idAddr()->iiaIGlabel->igNum);
             else
-                printf("L_M%03u_" FMT_BB, Compiler::s_compMethodsCount, id->idAddr()->iiaBBlabel->bbNum);
+                printf("L_M%03u_" FMT_BB, emitComp->compMethodID, id->idAddr()->iiaBBlabel->bbNum);
         }
         break;
 
index ec49e09..9702b73 100644 (file)
@@ -11164,11 +11164,11 @@ void emitter::emitDispIns(
             }
             else if (id->idIsBound())
             {
-                printf("G_M%03u_IG%02u", Compiler::s_compMethodsCount, id->idAddr()->iiaIGlabel->igNum);
+                printf("G_M%03u_IG%02u", emitComp->compMethodID, id->idAddr()->iiaIGlabel->igNum);
             }
             else
             {
-                printf("L_M%03u_" FMT_BB, Compiler::s_compMethodsCount, id->idAddr()->iiaBBlabel->bbNum);
+                printf("L_M%03u_" FMT_BB, emitComp->compMethodID, id->idAddr()->iiaBBlabel->bbNum);
             }
         }
         break;
@@ -11202,11 +11202,11 @@ void emitter::emitDispIns(
             emitDispReg(id->idReg1(), size, true);
             if (id->idIsBound())
             {
-                printf("G_M%03u_IG%02u", Compiler::s_compMethodsCount, id->idAddr()->iiaIGlabel->igNum);
+                printf("G_M%03u_IG%02u", emitComp->compMethodID, id->idAddr()->iiaIGlabel->igNum);
             }
             else
             {
-                printf("L_M%03u_" FMT_BB, Compiler::s_compMethodsCount, id->idAddr()->iiaBBlabel->bbNum);
+                printf("L_M%03u_" FMT_BB, emitComp->compMethodID, id->idAddr()->iiaBBlabel->bbNum);
             }
             break;
 
@@ -11216,11 +11216,11 @@ void emitter::emitDispIns(
             emitDispImm(emitGetInsSC(id), true);
             if (id->idIsBound())
             {
-                printf("G_M%03u_IG%02u", Compiler::s_compMethodsCount, id->idAddr()->iiaIGlabel->igNum);
+                printf("G_M%03u_IG%02u", emitComp->compMethodID, id->idAddr()->iiaIGlabel->igNum);
             }
             else
             {
-                printf("L_M%03u_" FMT_BB, Compiler::s_compMethodsCount, id->idAddr()->iiaBBlabel->bbNum);
+                printf("L_M%03u_" FMT_BB, emitComp->compMethodID, id->idAddr()->iiaBBlabel->bbNum);
             }
             break;
 
@@ -11277,11 +11277,11 @@ void emitter::emitDispIns(
                 }
                 else if (id->idIsBound())
                 {
-                    printf("G_M%03u_IG%02u", Compiler::s_compMethodsCount, id->idAddr()->iiaIGlabel->igNum);
+                    printf("G_M%03u_IG%02u", emitComp->compMethodID, id->idAddr()->iiaIGlabel->igNum);
                 }
                 else
                 {
-                    printf("L_M%03u_" FMT_BB, Compiler::s_compMethodsCount, id->idAddr()->iiaBBlabel->bbNum);
+                    printf("L_M%03u_" FMT_BB, emitComp->compMethodID, id->idAddr()->iiaBBlabel->bbNum);
                 }
             }
             printf("]");
index 38a3fff..6a21699 100644 (file)
@@ -7746,7 +7746,7 @@ void emitter::emitDispAddrMode(instrDesc* id, bool noDetail)
             {
                 printf("reloc ");
             }
-            printf("J_M%03u_DS%02u", Compiler::s_compMethodsCount, id->idDebugOnlyInfo()->idMemCookie);
+            printf("J_M%03u_DS%02u", emitComp->compMethodID, id->idDebugOnlyInfo()->idMemCookie);
 
             disp -= id->idDebugOnlyInfo()->idMemCookie;
         }
@@ -7884,7 +7884,7 @@ void emitter::emitDispAddrMode(instrDesc* id, bool noDetail)
 #else
 #define SIZE_LETTER "D"
 #endif
-        printf("\n\n    J_M%03u_DS%02u LABEL   " SIZE_LETTER "WORD", Compiler::s_compMethodsCount, jtno);
+        printf("\n\n    J_M%03u_DS%02u LABEL   " SIZE_LETTER "WORD", emitComp->compMethodID, jtno);
 
         /* Display the label table (it's stored as "BasicBlock*" values) */
 
@@ -7897,7 +7897,7 @@ void emitter::emitDispAddrMode(instrDesc* id, bool noDetail)
             lab = (insGroup*)emitCodeGetCookie(*bbp++);
             assert(lab);
 
-            printf("\n            D" SIZE_LETTER "      G_M%03u_IG%02u", Compiler::s_compMethodsCount, lab->igNum);
+            printf("\n            D" SIZE_LETTER "      G_M%03u_IG%02u", emitComp->compMethodID, lab->igNum);
         } while (--cnt);
     }
 }
@@ -8961,11 +8961,11 @@ void emitter::emitDispIns(
 
             if (id->idIsBound())
             {
-                printf("G_M%03u_IG%02u", Compiler::s_compMethodsCount, id->idAddr()->iiaIGlabel->igNum);
+                printf("G_M%03u_IG%02u", emitComp->compMethodID, id->idAddr()->iiaIGlabel->igNum);
             }
             else
             {
-                printf("L_M%03u_" FMT_BB, Compiler::s_compMethodsCount, id->idAddr()->iiaBBlabel->bbNum);
+                printf("L_M%03u_" FMT_BB, emitComp->compMethodID, id->idAddr()->iiaBBlabel->bbNum);
             }
             break;