Change ReadOnlySpan indexer to return ref readonly #14727

ahsonkhan · 2017-10-27T20:26:19Z

Fixes https://github.com/dotnet/corefx/issues/24105

Related PR: dotnet/corefx#24929

cc @ektrah, @VSadov, @jkotas, @KrzysztofCwalina

VSadov

LGTM

VSadov · 2017-10-27T20:34:01Z

CC: @jaredpar

jkotas · 2017-10-27T20:36:38Z

Have you tested this that it works? I think that this will need adjustments in the JIT and maybe VM as well - because of Span indexer is a very special intrinsic.

ahsonkhan · 2017-10-27T20:54:26Z

Have you tested this that it works?

I ran our unit tests locally, which passed (for netfx). What type of testing were you thinking? Peformance?

jkotas · 2017-10-27T21:01:45Z

That's surprising. Let's wait what the CI says...

VSadov · 2017-10-27T21:14:20Z

It looks like CI is failing due to missing/mismatching intrinsics.
I would assume the JIT intrinsics for ReadOnlySpan indexer now would be exactly the same as for the ordinary Span. So should be fixed relatively easily. Once one knows where to look :-)

AndyAyersMS · 2017-10-27T21:30:28Z

Most of the jit code is common for handling Span and ReadOnlySpan indexers. There's just a small difference in how the jit handles these, so yes, it would be easy to make them the same.

AndyAyersMS · 2017-10-27T22:02:50Z

Note, if you end up changing the jit, you probably should also make some changes in the runtime:

allocate a different intrinsic ID for this new form of read only span indexer and retire the old one
[perhaps] create a new jit GUID

Changes in corinfo.h require extra work to propagate to desktop.

ektrah · 2017-10-27T22:09:13Z

BTW, has the intrinsic already been shipped as part of .NET Core 2.0? If yes, how will the .NET Core 2.0 JIT react if the signature of the method changes in .NET Core 2.1?

AndyAyersMS · 2017-10-27T22:14:49Z

In the shipping product, the jit, the runtime, and corelib all match. So the 2.0 jit will never see the IL from a 2.1 corelib.

We (mainly the jit team) sometimes intentionally mismatch parts during development and testing and that's why we have some of these defensive mechanisms around like the jit GUID.

jkotas · 2017-10-27T22:32:21Z

[perhaps] create a new jit GUID

I would think that if you create a new jit GUID, you should not need to change the intrinsic ID. (Note that this intrinsic is not used by desktop.)

ektrah · 2017-10-27T22:32:46Z

@AndyAyersMS Ah, I forgot that ReadOnlySpan<T> has a copy in the corlib. But I'm referencing the System.Memory NuGet package, so I believe that my IL will expect a ref readonly T return type, even when it runs on 2.0. Is it OK that the 2.0 corelib doesn't provide that?

jkotas · 2017-10-27T22:33:50Z

@ektrah System.Memory package has not shipped a stable version yet. We are not guaranteeing any sort of backward compatibility for the preview versions.

ektrah · 2017-10-27T22:45:39Z

Hence my question if the intrinsic has already been shipped in 2.0 or if it will be first ship in 2.1. (Sorry, I'm not fully up to date with the internals.)

AndyAyersMS · 2017-10-27T23:26:54Z

Proposed jit changes here: AndyAyersMS/coreclr@83f166a

ahsonkhan · 2017-10-28T02:14:18Z

Any ideas what is causing these errors?

Unhandled Exception: System.MissingMethodException: Method not found: '!0 System.ReadOnlySpan`1.get_Item(Int32)'.

Do we need to update something in /vm/mscorlib.h:
https://github.com/dotnet/coreclr/blob/master/src/vm/mscorlib.h#L620

jkotas · 2017-10-28T06:39:39Z

All tests are crashing with access violation. Run any test under debugger (e.g. windbg on Windows). You should see it crash. Share stacktrace of the crash if you are not able to figure out the problem.

jkotas · 2017-10-28T06:44:32Z

Unhandled Exception: System.MissingMethodException: Method not found: '!0 System.ReadOnlySpan`1.get_Item(Int32)

This is another problem (different from the crash). The CoreFX depends on the old method in number of places. Everything that depends on the old method will break. You may need to build a private CoreFX package with the change, publish them to myget and point CoreCLR to this package until the change propagates through the system. Otherwise, the CoreCLR repo is going to be on the floor until the change propagates through the system.

ahsonkhan · 2017-11-01T22:04:15Z

The CoreFX depends on the old method in number of places. Everything that depends on the old method will break. You may need to build a private CoreFX package with the change, publish them to myget and point CoreCLR to this package until the change propagates through the system. Otherwise, the CoreCLR repo is going to be on the floor until the change propagates through the system.

I do not understand how to break this circular dependency. What change do I need to make to the private CoreFX package? Also, is this the property I need to update (i.e. MicrosoftPrivateCoreFxNETCoreAppPackageVersion) once the private package is up on myget?

ahsonkhan · 2017-11-01T22:04:27Z

@dotnet-bot test this please

ahsonkhan · 2017-11-01T23:32:57Z

All tests are crashing with access violation. Run any test under debugger (e.g. windbg on Windows). You should see it crash. Share stacktrace of the crash if you are not able to figure out the problem.

jkotas · 2017-11-02T18:07:47Z

What change do I need to make to the private CoreFX package?

You may want to chat with @weshaggard on how to best execute this. Also, do not forget to include ProjectN in the plan.

jkotas · 2017-11-02T18:09:22Z

The stacktrace you have shared does not seem to be related to the problem. You should see the crash even when you run a simple hello world - try to reproduce it on a hello world.

weshaggard · 2017-11-03T00:12:52Z

Wow this is a difficult one to stage. I'm a little nervous about using a private package as it will be hard to keep it in sync. Can you build the corefx changes without the CoreCLR change? or will the same thing happen in the CoreFx repo with tests failing?.

I suggest starting with creating the corefx PR as well and then we can decide how to proceed.

ahsonkhan · 2017-11-03T00:13:50Z

Can you build the corefx changes without the CoreCLR change? or will the same thing happen in the CoreFx repo with tests failing?.
I suggest starting with creating the corefx PR as well and then we can decide how to proceed.

It is already there and it depends on CoreCLR to pass CI: dotnet/corefx#24929

ahsonkhan · 2017-11-03T00:44:37Z

The stacktrace you have shared does not seem to be related to the problem. You should see the crash even when you run a simple hello world - try to reproduce it on a hello world.

I ran the Compilation test (which is HelloWorld) and didn't see an AV exception. Just the MissingMethod exception.

D:\GitHub\Fork\coreclr\tests\src\managed\Compilation>"D:\GitHub\Fork\coreclr\bin\tests\Windows_NT.x64.Debug\managed\Compilation\Compilation\Compilation.cmd" -debug D:\Debuggers\windbg.exe

CLR exception type: System.MissingMethodException
    "Method not found: '!0 System.ReadOnlySpan`1.get_Item(Int32)'."
*** WARNING: Unable to verify checksum for D:\GitHub\Fork\coreclr\bin\tests\Windows_NT.x64.Debug\Tests\Core_Root\CoreCLR.dll
KERNELBASE!RaiseException+0x68:
00007fff`11943fb8 488b8c24c0000000 mov     rcx,qword ptr [rsp+0C0h] ss:000000b7`52975580=000026fb78737d51
0:000> k
 # Child-SP          RetAddr           Call Site
00 000000b7`529754c0 00007ffe`bfa5da93 KERNELBASE!RaiseException+0x68 [minkernel\kernelbase\xcpt.c @ 922] 
01 000000b7`529755a0 00007ffe`bfa5e24e CoreCLR!__RethrowException+0x33 [f:\dd\vctools\crt\vcruntime\src\eh\frame.cpp @ 1136] 
02 000000b7`529755d0 00007fff`148941c3 CoreCLR!__CxxCallCatchBlock+0x19e [f:\dd\vctools\crt\vcruntime\src\eh\frame.cpp @ 1200] 
03 000000b7`52975670 00007ffe`bf221cf6 ntdll!RcFrameConsolidation+0x3 [minkernel\ntos\rtl\amd64\capture.asm @ 653] 
04 000000b7`5297c130 00007ffe`bf2221ca CoreCLR!MethodDesc::JitCompileCodeLocked+0x2b6 [d:\github\fork\coreclr\src\vm\prestub.cpp @ 836] 
05 000000b7`5297c360 00007ffe`bf2219b5 CoreCLR!MethodDesc::JitCompileCodeLockedEventWrapper+0x34a [d:\github\fork\coreclr\src\vm\prestub.cpp @ 702] 
06 000000b7`5297c560 00007ffe`bf223ca7 CoreCLR!MethodDesc::JitCompileCode+0x545 [d:\github\fork\coreclr\src\vm\prestub.cpp @ 642] 
07 000000b7`5297c730 00007ffe`bf223777 CoreCLR!MethodDesc::PrepareILBasedCode+0x217 [d:\github\fork\coreclr\src\vm\prestub.cpp @ 310] 
08 000000b7`5297c840 00007ffe`bf223ec5 CoreCLR!MethodDesc::PrepareCode+0x1e7 [d:\github\fork\coreclr\src\vm\prestub.cpp @ 292] 
09 000000b7`5297c950 00007ffe`bf21b625 CoreCLR!MethodDesc::PrepareInitialCode+0x1d5 [d:\github\fork\coreclr\src\vm\prestub.cpp @ 248] 
0a 000000b7`5297caa0 00007ffe`bf227c4a CoreCLR!MethodDesc::DoPrestub+0xa55 [d:\github\fork\coreclr\src\vm\prestub.cpp @ 1767] 
0b 000000b7`5297cd40 00007ffe`bf506d55 CoreCLR!PreStubWorker+0x45a [d:\github\fork\coreclr\src\vm\prestub.cpp @ 1517] 
0c 000000b7`5297cfe0 00007ffe`605188be CoreCLR!ThePreStub+0x55 [D:\GitHub\Fork\coreclr\src\vm\amd64\ThePreStubAMD64.asm @ 22] 
0d 000000b7`5297d090 00007ffe`605187f3 system_runtime_extensions!System.IO.StreamWriter.WriteLine(System.String)+0x5e*** WARNING: Unable to verify checksum for D:\GitHub\Fork\coreclr\bin\tests\Windows_NT.x64.Debug\Tests\Core_Root\system.runtime.extensions.dll
 [E:\A\_work\1485\s\corefx\src\System.Runtime.Extensions\src\System\IO\StreamWriter.cs @ 449] 
0e 000000b7`5297d100 00007ffe`60515bed system_console!System.IO.SyncTextWriter.WriteLine(System.String)+0x43*** WARNING: Unable to verify checksum for D:\GitHub\Fork\coreclr\bin\tests\Windows_NT.x64.Debug\Tests\Core_Root\system.console.dll
 [E:\A\_work\1485\s\corefx\src\System.Console\src\System\IO\SyncTextWriter.cs @ 280] 
0f 000000b7`5297d150 00007ffe`6051593d system_console!System.Console.WriteLine(System.String)+0x1d [E:\A\_work\1485\s\corefx\src\System.Console\src\System\Console.cs @ 555] 
10 000000b7`5297d180 00007ffe`bf76e033 Compilation!Program.Main(System.String[])+0x4d*** WARNING: Unable to verify checksum for D:\GitHub\Fork\coreclr\bin\tests\Windows_NT.x64.Debug\managed\Compilation\Compilation\Compilation.exe
 [D:\GitHub\Fork\coreclr\tests\src\managed\Compilation\Compilation.cs @ 17] 
11 000000b7`5297d2b0 00007ffe`bf3bd91e CoreCLR!CallDescrWorkerInternal+0x83 [D:\GitHub\Fork\coreclr\src\vm\amd64\CallDescrWorkerAMD64.asm @ 101] 
12 000000b7`5297d2f0 00007ffe`bf3be72a CoreCLR!CallDescrWorkerWithHandler+0x1de [d:\github\fork\coreclr\src\vm\callhelpers.cpp @ 81] 
13 000000b7`5297d370 00007ffe`befa26f4 CoreCLR!MethodDescCallSite::CallTargetWorker+0xdfa [d:\github\fork\coreclr\src\vm\callhelpers.cpp @ 648] 
14 000000b7`5297d9e0 00007ffe`befdc5a9 CoreCLR!MethodDescCallSite::Call_RetArgSlot+0xf4 [d:\github\fork\coreclr\src\vm\callhelpers.h @ 433] 
15 000000b7`5297dae0 00007ffe`befdc6e5 CoreCLR!``RunMain'::`30'::__Body::Run'::`5'::__Body::Run+0x269 [d:\github\fork\coreclr\src\vm\assembly.cpp @ 1709] 
16 000000b7`5297dc90 00007ffe`befdc9ee CoreCLR!`RunMain'::`30'::__Body::Run+0x85 [d:\github\fork\coreclr\src\vm\assembly.cpp @ 1720] 
17 000000b7`5297dd10 00007ffe`befd3112 CoreCLR!RunMain+0x19e [d:\github\fork\coreclr\src\vm\assembly.cpp @ 1720] 
18 000000b7`5297ddb0 00007ffe`bee9396e CoreCLR!Assembly::ExecuteMainMethod+0x412 [d:\github\fork\coreclr\src\vm\assembly.cpp @ 1802] 
*** WARNING: Unable to verify checksum for CoreRun.exe
19 000000b7`5297e270 00007ff6`5cf55b25 CoreCLR!CorHost2::ExecuteAssembly+0x54e [d:\github\fork\coreclr\src\vm\corhost.cpp @ 495] 
1a 000000b7`5297e540 00007ff6`5cf579bb CoreRun!TryRun+0xcb5 [d:\github\fork\coreclr\src\coreclr\hosts\corerun\corerun.cpp @ 581] 
1b 000000b7`5297f7e0 00007ff6`5d0140f4 CoreRun!wmain+0x14b [d:\github\fork\coreclr\src\coreclr\hosts\corerun\corerun.cpp @ 696] 
1c 000000b7`5297f880 00007ff6`5d013fa7 CoreRun!invoke_main+0x34 [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 91] 
1d 000000b7`5297f8c0 00007ff6`5d013e6e CoreRun!__scrt_common_main_seh+0x127 [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 283] 
1e 000000b7`5297f920 00007ff6`5d014189 CoreRun!__scrt_common_main+0xe [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 326] 
1f 000000b7`5297f950 00007fff`11e61fe4 CoreRun!wmainCRTStartup+0x9 [f:\dd\vctools\crt\vcstartup\src\startup\exe_wmain.cpp @ 17] 
20 000000b7`5297f980 00007fff`1485ef91 KERNEL32!BaseThreadInitThunk+0x14 [base\win32\client\thread.c @ 64] 
21 000000b7`5297f9b0 00000000`00000000 ntdll!RtlUserThreadStart+0x21 [minkernel\ntdll\rtlstrt.c @ 997]

jkotas · 2017-11-03T00:55:57Z

Windows_NT.x64.Debug

The CI failures are on checked build. You may want to try that - there may be JIT bug that repros on checked build only.

…e ReadOnlySpan indexer to return ref readonly. Related to https://github.com/dotnet/corefx/issues/24105 / dotnet/coreclr#14727. [tfs-changeset: 1683794]

ahsonkhan · 2017-12-14T03:53:41Z

Found the issue:
https://github.com/dotnet/coreclr/blob/master/tests/src/JIT/superpmi/superpmicollect.cs#L346

string[] mcFiles = Directory.GetFiles(s_tempDir, "*.mc");

GetFiles ends up calling PathHelpers.GetDirectoryNameNoChecks eventually (along with CombineNoChecksInternal/etc.) which uses the ReadOnlySpan indexer (partially due to the changes in here - dotnet/corefx#25426).

These recent changes should resolve this issue:
dotnet/corefx#25881
dotnet/corefx#25908

ahsonkhan · 2017-12-14T03:54:18Z

@dotnet-bot test this please

jkotas · 2017-12-14T15:54:32Z

@ahsonkhan Could you please resolve the conflict?

jkotas · 2017-12-14T18:48:16Z

(And maybe squash to a fewer commits as well.)

Update JIT to handle changes to ReadOnlySpan indexer Resolving merge conflict and fixing jit importer Update ReadOnlySpan Enumerator Current to use indexer. Removing readonly keyword.

Reverting the change to Enumerator Current for now Fix file formatting Enable Alpine CI (dotnet#15502) * Enable Alpine CI This enables Alpine CI leg on every PR using the pipelines. compare type size instead of var_types get rid of TYP_CHAR Adding support for Acosh, Asinh, Atanh, and Cbrt to Math and MathF Updating the PAL layer to support acosh, asinh, atanh, and cbrt Adding some PAL tests for acosh, asinh, atanh, and cbrt Adding valuenum support for acosh, asinh, atanh, and cbrt Lsra Documentation Update LinearScan section of ryujit-overview.md, and add lsra-detail.md Refactor Unsafe.cs to get it more in sync with CoreRT. (dotnet#15510) * Refactor Unsafe.cs to get it more in sync with CoreRT. * Format the document. * Unifying the copies of Unsafe using ifdefs * Change exception thrown to PlatformNotSupportedException * Addressing PR feedback and moving Unsafe to shared. * Addressing PR feedback * Addressing PR review - adding intrinsic attribute Update CoreClr, CoreFx to preview1-26014-01, preview1-26013-12, respectively (dotnet#15513) Revert "Add optional integer offset to OwnedMemory Pin (dotnet#15410)" This reverts commit 8931cfa. Get rid of old -altjitcrossgen argument now that CI has been updated Merge pull request dotnet/corert#5109 from dotnet/nmirror (dotnet#15518) Merge nmirror to master Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com> Revert " Revert "[Local GC] Move knowledge of overlapped I/O objects to the EE through four callbacks (dotnet#14982)"" Fix typo `_TARGET_ARM` to `_TARGET_ARM_`. This happens mostly in comments except lsra.cpp. Update CoreClr, CoreFx, PgoData to preview1-26014-04, preview1-26014-03, master-20171214-0043, respectively (dotnet#15520)

jkotas · 2017-12-14T22:52:58Z

src/mscorlib/shared/System/ReadOnlySpan.cs

@@ -175,7 +175,7 @@ public bool IsEmpty
            }
        }
 #else
-        public T this[int index]
+        public ref readonly T this[int index]
        {


Nit: You can move the #if PROJECTN ifdef back here (so that it looks similar to Span.cs).

jkotas · 2017-12-14T23:44:21Z

tests/src/JIT/superpmi/superpmicollect.cs

@@ -199,17 +205,20 @@ private static void RunTest(string testName)

            if (Global.IsWindows)
            {
+                Console.WriteLine("================O=========="); 


Did you meant to keep these debug statements?

Nope. Will remove.

ahsonkhan · 2017-12-15T01:33:28Z

We have some JIT Hardware Intrinsic tests failing here:
https://ci.dot.net/job/dotnet_coreclr/job/master/job/checked_osx10.12_tst_prtest/8231/consoleFull#-756853549f66db340-b3cd-4e23-8190-9b52cb916333

Is this a known issue? Are these failures intermittent? cc @BruceForstall, @CarolEdit?

17:18:11 FAILED   - JIT/HardwareIntrinsics/Add_r/Add_r.sh
17:18:11                BEGIN EXECUTION
17:18:11                /Users/dotnet-bot/j/workspace/dotnet_coreclr/master/checked_osx10.12_tst_prtest/bin/tests/OSX.x64.Checked/Tests/Core_Root/corerun Add_r.exe
17:18:11                
17:18:11 
               Assert failure(PID 90570 [0x000161ca], Thread: 3236629 [0x316315]): Assertion failed 'dataSrc->OperIsIndir()' in 'System.Runtime.Intrinsics.X86.Avx:Add(struct,struct):struct' (IL size 8)
17:18:11                
17:18:11                    File: /Users/dotnet-bot/j/workspace/dotnet_coreclr/master/checked_osx10.12_prtest/src/jit/rationalize.cpp Line: 853
17:18:11                    Image: /Users/dotnet-bot/j/workspace/dotnet_coreclr/master/checked_osx10.12_tst_prtest/bin/tests/OSX.x64.Checked/Tests/Core_Root/corerun
17:18:11                
17:18:11                ./Add_r.sh: line 243: 90570 Abort trap: 6           (core dumped) $_DebuggerFullPath "$CORE_ROOT/corerun" $ExePath $CLRTestExecutionArguments
17:18:11                Expected: 100
17:18:11                Actual: 134
17:18:11                END EXECUTION - FAILED
17:18:11 FAILED   - JIT/HardwareIntrinsics/Add_ro/Add_ro.sh
17:18:11                BEGIN EXECUTION
17:18:11                /Users/dotnet-bot/j/workspace/dotnet_coreclr/master/checked_osx10.12_tst_prtest/bin/tests/OSX.x64.Checked/Tests/Core_Root/corerun Add_ro.exe
17:18:11                
17:18:11                Assert failure(PID 90595 [0x000161e3], Thread: 3236681 [0x316349]): Assertion failed 'genActualType(lclTyp) == genActualType(op1->gtType) || genActualType(lclTyp) == TYP_I_IMPL && op1->IsVarAddr() || (genActualType(lclTyp) == TYP_I_IMPL && (op1->gtType == TYP_BYREF || op1->gtType == TYP_REF)) || (genActualType(op1->gtType) == TYP_I_IMPL && lclTyp == TYP_BYREF) || (varTypeIsFloating(lclTyp) && varTypeIsFloating(op1->TypeGet())) || ((genActualType(lclTyp) == TYP_BYREF) && genActualType(op1->TypeGet()) == TYP_REF) : Possibly bad IL with CEE_stloc.s at offset 008Ah (op1=simd32 op2=NULL stkDepth=0)' in 'IntelHardwareIntrinsicTest.Program:Main(ref):int' (IL size 4483)
17:18:11                
17:18:11                    File: /Users/dotnet-bot/j/workspace/dotnet_coreclr/master/checked_osx10.12_prtest/src/jit/importer.cpp Line: 10693
17:18:11                    Image: /Users/dotnet-bot/j/workspace/dotnet_coreclr/master/checked_osx10.12_tst_prtest/bin/tests/OSX.x64.Checked/Tests/Core_Root/corerun
17:18:11                
17:18:11                ./Add_ro.sh: line 243: 90595 Abort trap: 6           (core dumped) $_DebuggerFullPath "$CORE_ROOT/corerun" $ExePath $CLRTestExecutionArguments
17:18:11                Expected: 100
17:18:11                Actual: 134
17:18:11                END EXECUTION - FAILED

ahsonkhan · 2017-12-15T01:33:39Z

@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test

jkotas · 2017-12-15T01:35:19Z

Yes, this is known issue: https://github.com/dotnet/coreclr/issues/15519

) * Change ReadOnlySpan indexer to return ref readonly. Update JIT to handle changes to ReadOnlySpan indexer Resolving merge conflict and fixing jit importer Update ReadOnlySpan Enumerator Current to use indexer. Removing readonly keyword. * Temporarily disabling Span perf and other tests that use ReadOnlySpan * Isolating the ref readonly indexer change only to CoreCLR for now. Reverting the change to Enumerator Current for now Fix file formatting Enable Alpine CI (#15502) * Enable Alpine CI This enables Alpine CI leg on every PR using the pipelines. compare type size instead of var_types get rid of TYP_CHAR Adding support for Acosh, Asinh, Atanh, and Cbrt to Math and MathF Updating the PAL layer to support acosh, asinh, atanh, and cbrt Adding some PAL tests for acosh, asinh, atanh, and cbrt Adding valuenum support for acosh, asinh, atanh, and cbrt Lsra Documentation Update LinearScan section of ryujit-overview.md, and add lsra-detail.md Refactor Unsafe.cs to get it more in sync with CoreRT. (#15510) * Refactor Unsafe.cs to get it more in sync with CoreRT. * Format the document. * Unifying the copies of Unsafe using ifdefs * Change exception thrown to PlatformNotSupportedException * Addressing PR feedback and moving Unsafe to shared. * Addressing PR feedback * Addressing PR review - adding intrinsic attribute Update CoreClr, CoreFx to preview1-26014-01, preview1-26013-12, respectively (#15513) Revert "Add optional integer offset to OwnedMemory Pin (#15410)" This reverts commit 8931cfa. Get rid of old -altjitcrossgen argument now that CI has been updated Merge pull request dotnet/corert#5109 from dotnet/nmirror (#15518) Merge nmirror to master Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com> Revert " Revert "[Local GC] Move knowledge of overlapped I/O objects to the EE through four callbacks (#14982)"" Fix typo `_TARGET_ARM` to `_TARGET_ARM_`. This happens mostly in comments except lsra.cpp. Update CoreClr, CoreFx, PgoData to preview1-26014-04, preview1-26014-03, master-20171214-0043, respectively (#15520) * Disabling a test that uses ReadOnlySpan indexer * Temporarily disabling the superpmi test and fixing nit * Remove debug statements. Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>

dnfclas added the cla-already-signed label Oct 27, 2017

ahsonkhan mentioned this pull request Oct 27, 2017

ReadOnlySpan indexer now returns ref readonly dotnet/corefx#24929

Closed

VSadov approved these changes Oct 27, 2017

View reviewed changes

jaredpar approved these changes Oct 27, 2017

View reviewed changes

ahsonkhan self-assigned this Dec 14, 2017

ahsonkhan added the area-System.Memory label Dec 14, 2017

ahsonkhan mentioned this pull request Dec 14, 2017

Mirror changes from dotnet/corert #15518

Merged

ahsonkhan force-pushed the SpanRefReadOnly branch from 8c398b2 to 7c18942 Compare December 14, 2017 19:24

ahsonkhan added 3 commits December 14, 2017 11:27

Change ReadOnlySpan indexer to return ref readonly.

46eb5b6

Update JIT to handle changes to ReadOnlySpan indexer Resolving merge conflict and fixing jit importer Update ReadOnlySpan Enumerator Current to use indexer. Removing readonly keyword.

Temporarily disabling Span perf and other tests that use ReadOnlySpan

7abd39d

ahsonkhan force-pushed the SpanRefReadOnly branch from 7c18942 to 8bc3964 Compare December 14, 2017 19:28

Disabling a test that uses ReadOnlySpan indexer

d7e04dd

jkotas reviewed Dec 14, 2017

View reviewed changes

Temporarily disabling the superpmi test and fixing nit

a8aaeb6

jkotas reviewed Dec 14, 2017

View reviewed changes

Remove debug statements.

7198b34

ahsonkhan merged commit 1ec4888 into dotnet:master Dec 15, 2017

ahsonkhan deleted the SpanRefReadOnly branch December 15, 2017 02:24

ahsonkhan mentioned this pull request Dec 15, 2017

Enabling temporarily disabled tests that call ReadOnlySpan indexer #15546

Merged

dlemstra mentioned this pull request Jan 23, 2018

Image.Load(byte[]) throws Method not found System.ReadOnlySpan.get_Item(Int32) SixLabors/ImageSharp#447

Closed

ahsonkhan mentioned this pull request Jan 31, 2020

Re-enable temporarily disabled tests that call ReadOnlySpan indexer dotnet/runtime#9289

Closed

Change ReadOnlySpan indexer to return ref readonly #14727

Change ReadOnlySpan indexer to return ref readonly #14727

Conversation

ahsonkhan commented Oct 27, 2017 • edited Loading

VSadov left a comment

Choose a reason for hiding this comment

VSadov commented Oct 27, 2017

jkotas commented Oct 27, 2017

ahsonkhan commented Oct 27, 2017 • edited Loading

jkotas commented Oct 27, 2017

VSadov commented Oct 27, 2017

AndyAyersMS commented Oct 27, 2017

AndyAyersMS commented Oct 27, 2017

ektrah commented Oct 27, 2017

AndyAyersMS commented Oct 27, 2017

jkotas commented Oct 27, 2017 • edited Loading

ektrah commented Oct 27, 2017

jkotas commented Oct 27, 2017 • edited Loading

ektrah commented Oct 27, 2017

AndyAyersMS commented Oct 27, 2017

ahsonkhan commented Oct 28, 2017 • edited Loading

jkotas commented Oct 28, 2017

jkotas commented Oct 28, 2017

ahsonkhan commented Nov 1, 2017 • edited Loading

ahsonkhan commented Nov 1, 2017

ahsonkhan commented Nov 1, 2017

jkotas commented Nov 2, 2017

jkotas commented Nov 2, 2017

weshaggard commented Nov 3, 2017

ahsonkhan commented Nov 3, 2017

ahsonkhan commented Nov 3, 2017 • edited Loading

jkotas commented Nov 3, 2017

ahsonkhan commented Dec 14, 2017 • edited Loading

ahsonkhan commented Dec 14, 2017

jkotas commented Dec 14, 2017

jkotas commented Dec 14, 2017

jkotas Dec 14, 2017

Choose a reason for hiding this comment

jkotas Dec 14, 2017

Choose a reason for hiding this comment

ahsonkhan Dec 14, 2017

Choose a reason for hiding this comment

ahsonkhan commented Dec 15, 2017

ahsonkhan commented Dec 15, 2017

jkotas commented Dec 15, 2017

ahsonkhan commented Oct 27, 2017 •

edited

Loading

ahsonkhan commented Oct 27, 2017 •

edited

Loading

jkotas commented Oct 27, 2017 •

edited

Loading

jkotas commented Oct 27, 2017 •

edited

Loading

ahsonkhan commented Oct 28, 2017 •

edited

Loading

ahsonkhan commented Nov 1, 2017 •

edited

Loading

ahsonkhan commented Nov 3, 2017 •

edited

Loading

ahsonkhan commented Dec 14, 2017 •

edited

Loading