Local GC: stop sharing GC globals between GC and write barrier

[Maoni0]

As part of the LocalGC project I’d like to stop sharing the GC globals between the GC code and the WB (Write Barrier) code. The globals are (note I am not including things that don’t run on customers machines by default like debug only code or GCShadow stuff):

g_card_table
g_lowest_address
g_highest_address
g_ephemeral_low
g_ephemeral_high

The last 2 are only changed when EE is suspended so there’s no significance in the order they are set wrt anything else – managed threads are not running at this point anyway. But it would still be good to not share them.

The first 3 are what causes all this intricacy. There is one invariant that we need to stick to which is

 The WB code must see the updated g_card_table before they see updated g_lowest/highest

So the WB side just needs to make sure they are updated in that order.

On the GC side, since GC only uses these either when EE is stopped or when it’s under a global lock. As long as we are not stopped in the middle of setting these 3 values we are fine (the rare race that occurs today is due to the fact that we can be stopped in the middle).

So instead of declaring these on the GC side, we should have 2 sets of copies for these, one on GC side; the other on CLR side -

Remove the global declaration of them from gccommon.cpp and add them on the CLR side.
Rename them on the GC side so it’s clear they are separate. Replace g_ to g_gc_.
Pass them in the StompWriteBarrierResize and StompWriteBarrierEphemeral which will be part of the to GCToEEInterface/IGCToCLR interface.

// Consider creating a struct that includes all the globals so we don't need to pass in so many params
void StompWriteBarrierResize(bool isRuntimeSuspended, bool bReqUpperBoundsCheck, void* new_card_table, void* new_lowest_address, void new_highest_address);

// I would just get rid of the isRuntimeSuspesnded parameter as it's never called with false.
void WriteBarrierManager::UpdateEphemeralBounds(void* new_ephemeral_low, void* new_ephemeral_high);

In GC grow_brick_card_tables is where we might need to change the first 3 values. Right now it has this sequence:

g_card_table = new_card_table;
StompWriteBarrierResize();
GCToOSInterface::FlushProcessWriteBuffers();
g_lowest_address = new_lowest_address;
VolatileStore(&g_highest_address, new_highest_address);

This will be changed to:

g_gc_card_table = new_card_table;
g_gc_lowest_address = new_lowest_address;
g_gc_highest_address = new_highest_address;

StompWriteBarrierResize(param0, param1, g_gc_card_table, g_gc_lowest_address, g_gc_highest_address);

And in StompWriteBarrierResize,

Do what StompWriteBarrierResize currently does (ie, might suspend EE to change WB type; bash the g_card_table value) and then

g_card_table = new_card_table;
FlushProcessWriteBuffers();
g_lowest_address = new_lowest_address;
VolatileStore(&g_highest_address, new_highest_address);

@swgillespie Note I didn't include the software WW stuff... feel free to add that as we discussed.

[swgillespie]

With regards to SWW, the address of the write watch table can be communicated in the same manner as g_card_table above. Today, the non-assembly write barriers call SoftwareWriteWatch::SetDirty to mark a page as dirty, while the assembly write barriers do it manually in the same manner as the card table. We can either expose SetDirty on the interface, or we can do what the the assembly does to mark a page as dirty (https://github.com/dotnet/coreclr/blob/master/src/vm/amd64/jithelpers_fast.S#L84), but in C++.

[swgillespie]

Related: https://github.com/dotnet/coreclr/issues/5741

[swgillespie]

fixed by dotnet/coreclr#8605 and dotnet/coreclr#8568

[xiangzhai]

:mips-interest

WriteBarrierManager is only for AMD64, and MIPS64 followed the ARM64 way in the very beginning of CoreCLR MIPS64 porting.

But MIPS64 recently begin to follow the UNIX AMD64 ABI, for example, "copy" the SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR to MIPS64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR for passing and returning struct in the registers. The side effect of the changing is GC could not work correctly, because AMD64 treat gcPtrs differently from ARM64.

I am (preparing) changing to AMD64 way...

Thanks,
Leslie Zhai

[janvorli]

@xiangzhai it is not clear to me how passing struct in registers is related to GC write barriers. Can you please be more specific on that? Maybe I can help you to figure out a way without changing to the amd64 way of handling write barriers.

[xiangzhai]

Hi @janvorli

We will open source ASAP :) https://github.com/gsvm

it is not clear to me how passing struct in registers is related to GC write barriers.

Sorry for my poor English! I just paste MIPS64 code about returning struct in the registers:

void ReturnTypeDesc::InitializeStructReturnType(Compiler* comp, CORINFO_CLASS_HANDLE retClsHnd)
{                                                                               
...
        case Compiler::SPK_ByValue:                                             
        {                                                                       
            assert(varTypeIsStruct(returnType));                                
                                                                                
#ifdef UNIX_AMD64_ABI                                                           
                                                                                
            SYSTEMV_AMD64_CORINFO_STRUCT_REG_PASSING_DESCRIPTOR structDesc;     
            comp->eeGetSystemVAmd64PassStructInRegisterDescriptor(retClsHnd, &structDesc);
                                                                                
            assert(structDesc.passedInRegisters);                               
            for (int i = 0; i < structDesc.eightByteCount; i++)                 
            {                                                                   
                assert(i < MAX_RET_REG_COUNT);                                  
                m_regType[i] = comp->GetEightByteType(structDesc, i);           
            }                                                                   
                                                                                
#elif defined(_TARGET_ARM64_)                                                   
                                                                                
            // a non-HFA struct returned using two registers                    
            //                                                                  
            assert((structSize > TARGET_POINTER_SIZE) && (structSize <= (2 * TARGET_POINTER_SIZE)));
                                                                                
            BYTE gcPtrs[2] = {TYPE_GC_NONE, TYPE_GC_NONE};                      
            comp->info.compCompHnd->getClassGClayout(retClsHnd, &gcPtrs[0]);    
            for (unsigned i = 0; i < 2; ++i)                                    
            {                                                                   
                m_regType[i] = comp->getJitGCType(gcPtrs[i]);                   
            }                                                                   
                                                                                
#elif defined(_TARGET_MIPS64_)                                                  
                                                                                
            // a non-HFA struct returned using two registers                    
            //                                                                  
            assert((structSize >= TARGET_POINTER_SIZE) && (structSize <= (2 * TARGET_POINTER_SIZE)));
                                                                                
            if (comp->isTwoFloatStruct(retClsHnd))                              
            {                                                                   
                m_regType[0] = TYP_FLOAT;                                       
                m_regType[1] = TYP_FLOAT;                                       
            }                                                                   
            else if (comp->isTwoDoubleStruct(retClsHnd))                        
            {                                                                   
                m_regType[0] = TYP_DOUBLE;                                      
                m_regType[1] = TYP_DOUBLE;                                      
            }                                                                   
            else                                                                
            {                                                                   
                BYTE gcPtrs[2] = {TYPE_GC_NONE, TYPE_GC_NONE};                  
                comp->info.compCompHnd->getClassGClayout(retClsHnd, &gcPtrs[0]);
                for (unsigned i = 0; i < 2; ++i)                                
                {                                                               
                    m_regType[i] = comp->getJitGCType(gcPtrs[i]);               
                }                                                               
            }                                                                   
...

We implemented isTwoFloatStruct and isTwoDoubleStruct to meet MIPS64 ABI, but it might lost TYP_REF or TYP_BYREF for GC types ...

And I persuaded myself not changed to AMD64 way because Loongson 3A3000 is weak memory model more likely ARM64.

Thanks,
Leslie Zhai

[janvorli]

It seems that for the twofloat / twodouble stuff you got it right. Floats and doubles cannot be ref nor byref.

[xiangzhai]

emm, let's wait our open source :)

The testcase is XUnit:

System.InvalidCastException: Unable to cast object of type 'Xunit.Sdk.ReflectionAssemblyInfo' to type 'Xunit.Abstractions.IAssemblyInfo' 

Workaround:

diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp
index 319d90e..1ec2905 100644
--- a/src/jit/gentree.cpp
+++ b/src/jit/gentree.cpp
@@ -18155,6 +18155,7 @@ void ReturnTypeDesc::InitializeStructReturnType(Compiler* comp, CORINFO_CLASS_HA
             //
             assert((structSize >= TARGET_POINTER_SIZE) && (structSize <= (2 * TARGET_POINTER_SIZE)));

+#if 0
             if (comp->isTwoFloatStruct(retClsHnd))
             {
                 m_regType[0] = TYP_FLOAT;
@@ -18166,6 +18167,7 @@ void ReturnTypeDesc::InitializeStructReturnType(Compiler* comp, CORINFO_CLASS_HA
                 m_regType[1] = TYP_DOUBLE;
             }
             else
+#endif
             {
                 BYTE gcPtrs[2] = {TYPE_GC_NONE, TYPE_GC_NONE};
                 comp->info.compCompHnd->getClassGClayout(retClsHnd, &gcPtrs[0]);

Then xunit was able to work after commented the isTwoFloatStruct and isTwoDoubleStruct.