Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Move Span/ReadOnlySpan to shared CoreLib partition#10988

Merged
jkotas merged 3 commits into
dotnet:masterfrom
jkotas:corert-span
Apr 15, 2017
Merged

Move Span/ReadOnlySpan to shared CoreLib partition#10988
jkotas merged 3 commits into
dotnet:masterfrom
jkotas:corert-span

Conversation

@jkotas
Copy link
Copy Markdown
Member

@jkotas jkotas commented Apr 15, 2017

Fix a few method names to better names used in CoreRT

Contributes to dotnet/corert#2966

Fix a few method names to better names used in CoreRT

Contributes to dotnet/corert#2966
private extern static bool IsAddressInStack(IntPtr ptr);
#endif

static internal bool ByRefLessThan<T>(ref T refA, ref T refB)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the rationale for removing this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not used anywhere.

Comment thread src/vm/mscorlib.h
DEFINE_FIELD(PINNING_HELPER, M_DATA, m_data)

DEFINE_CLASS(ARRAY_PINNING_HELPER, CompilerServices, ArrayPinningHelper)
DEFINE_FIELD(ARRAY_PINNING_HELPER, M_ARRAY_DATA, m_arrayData)
Copy link
Copy Markdown

@ahsonkhan ahsonkhan Apr 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this is causing CI build errors. Also it is used here:

mdToken tokArrayPinningHelper = MscorlibBinder::GetField(FIELD__ARRAY_PINNING_HELPER__M_ARRAY_DATA)->GetMemberDef();

d:\j\workspace\debug_windows---17180f1b\src\vm\jitinterface.cpp(6926): error C2065: 'FIELD__ARRAY_PINNING_HELPER__M_ARRAY_DATA': undeclared identifier [D:\j\workspace\debug_windows---17180f1b\bin\obj\Windows_NT.x64.Debug\src\vm\crossgen\cee_crossgen.vcxproj]

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidental change ... fixed.


internal static class SpanHelper
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we separate the Span/SpanHelper class into a separate file, esp since it has internal statics used by both Span and ReadOnlySpan?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree.

}

static internal ref T GetArrayData<T>(T[] array)
static internal ref byte GetRawSzArrayData(this Array array)
Copy link
Copy Markdown

@ahsonkhan ahsonkhan Apr 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

METHOD__JIT_HELPERS__GET_ARRAY_DATA and
METHOD__JIT_HELPERS__GET_RAW_SZ_ARRAY_DATA case both point to the same code and yet one returns a ref T while the other returns a ref byte? How does that work?

    else if (tk == MscorlibBinder::GetMethod(METHOD__JIT_HELPERS__GET_ARRAY_DATA)->GetMemberDef())
    {
        mdToken tokArrayPinningHelper = MscorlibBinder::GetField(FIELD__ARRAY_PINNING_HELPER__M_ARRAY_DATA)->GetMemberDef();

        static BYTE ilcode[] = { CEE_LDARG_0,
                                 CEE_LDFLDA,0,0,0,0,
                                 CEE_RET };

        ilcode[2] = (BYTE)(tokArrayPinningHelper);
        ilcode[3] = (BYTE)(tokArrayPinningHelper >> 8);
        ilcode[4] = (BYTE)(tokArrayPinningHelper >> 16);
        ilcode[5] = (BYTE)(tokArrayPinningHelper >> 24);

        methInfo->ILCode = const_cast<BYTE*>(ilcode);
        methInfo->ILCodeSize = sizeof(ilcode);
        methInfo->maxStack = 1;
        methInfo->EHcount = 0;
        methInfo->options = (CorInfoOptions)0;
        return true;
    }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IL is same for both cases because of it depends on unsafe casts.

ThrowHelper.ThrowArgumentOutOfRangeException();

_pointer = new ByReference<T>(ref Unsafe.Add(ref JitHelpers.GetArrayData(array), start));
_pointer = new ByReference<T>(ref Unsafe.Add(ref Unsafe.As<byte, T>(ref array.GetRawSzArrayData()), start));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does having to pass ref array.GetRawSzArrayData() to Unsafe.As<byte, T> incur significant performance cost?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JITed code is the same in both cases.

@ahsonkhan
Copy link
Copy Markdown

@dotnet-bot test Ubuntu16.04 arm Cross Debug Build

@ahsonkhan
Copy link
Copy Markdown

@dotnet-bot test Ubuntu arm Cross Release Build

@ahsonkhan
Copy link
Copy Markdown

LGTM

@jkotas jkotas merged commit e0b8c96 into dotnet:master Apr 15, 2017
dotnet-bot pushed a commit to dotnet/corert that referenced this pull request Apr 15, 2017
)

Fix a few method names to better names used in CoreRT

Contributes to #2966

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
@jkotas jkotas deleted the corert-span branch April 17, 2017 03:33
jkotas added a commit to dotnet/corert that referenced this pull request Apr 17, 2017
)

Fix a few method names to better names used in CoreRT

Contributes to #2966

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
dotnet-bot pushed a commit to dotnet/corefx that referenced this pull request Jan 13, 2018
)

Fix a few method names to better names used in CoreRT

Contributes to dotnet/corert#2966

Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
dotnet-bot pushed a commit to dotnet/corefx that referenced this pull request Jan 13, 2018
)

Fix a few method names to better names used in CoreRT

Contributes to dotnet/corert#2966

Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
safern pushed a commit to dotnet/corefx that referenced this pull request Jan 16, 2018
)

Fix a few method names to better names used in CoreRT

Contributes to dotnet/corert#2966

Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
safern pushed a commit to dotnet/corefx that referenced this pull request Jan 16, 2018
)

Fix a few method names to better names used in CoreRT

Contributes to dotnet/corert#2966

Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants