Revert "Fix NativeAOT ThunksPool thunk data block size handling."

[jkotas]

Reverts #110732

[jkotas]

/azp run runtime-nativeaot-outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

[MichalStrehovsky]

The only outerloop failure looks to be the known CRT bug (since we finished the test and then crashed on shutdown - I'm not going to look at the dump). Reminds me to look into #109006 once I'm back at work.

[MichalStrehovsky]

@lateralusX are the ARM32 failures something you could look into this week? Otherwise I'd prefer we merge the revert so that native AOT outerloop can be green again. It's very difficult to watch for regressions when every outerloop run is red. See #110852 (comment) for more context.

[MichalStrehovsky]

@lateralusX are the ARM32 failures something you could look into this week? Otherwise I'd prefer we merge the revert so that native AOT outerloop can be green again. It's very difficult to watch for regressions when every outerloop run is red. See #110852 (comment) for more context.

This keeps generating noise, such as in #110828 CI run. Reverting.

[lateralusX]

@lateralusX are the ARM32 failures something you could look into this week? Otherwise I'd prefer we merge the revert so that native AOT outerloop can be green again. It's very difficult to watch for regressions when every outerloop run is red. See #110852 (comment) for more context.

This keeps generating noise, such as in #110828 CI run. Reverting.

OK, I can take a look during the week and see how we could get the ARM32 outloop tests working with this fix.

@jkotas my original suggestion, 200f3aa, didn't have this issue since it returns the actual size of the matching thunk data block size from the source using separate functions in asm/cpp implementation instead of doing the calculation in managed code that could end up with a mismatch if the size of the data thunk is not close to the size of the code thunk ending up with a thunk data block size that will be to small since num of thunks per block will reflect number of code thunks and not data thunks, but since data thunk is only 8 bytes (two pointers) on 32-bit, it will probably end up as 204 (NumThunksPerBlock code thunk size 20 and page size 4096) * 8 (ThunkDataSize) + 4 (IntPtr.Size) = 1636 and then doing RoundUpToPowerOf2 would only put us up to 2048, while the page size is probably 4096, meaning we won't update common stub address that is left at 0.

Since the calculation of RhpGetNumThunksPerBlock seems to be the min of number of code or data thunks that fits into a page (similar logic included in all asm implementations as well):

FCIMPL0(int, RhpGetNumThunksPerBlock)
{
    return min(
        OS_PAGE_SIZE / THUNK_SIZE,                              // Number of thunks that can fit in a page
        (OS_PAGE_SIZE - POINTER_SIZE) / (POINTER_SIZE * 2)      // Number of pointer pairs, minus the jump stub cell, that can fit in a page
    );
}
FCIMPLEND

We would need to know these details to calculate the correct thunk data block size in managed code, meaning we probably need to take max of THUNK_SIZE and POINTER_SIZE * 2 in managed code and use that to calculate the size of code and data thunk block. If it is acceptable to add this logic in managed code I believe we could just tweak the PR with that change, alternative is to move back to the initial commit adding explicit functions to return the size of the data thunk block.

[lateralusX]

New attempt, #111149.