Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update SVE instructions that writes to GC regs #112389

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

kunalspathak
Copy link
Member

We didn't update the method emitInsMayWriteToGCReg when we added SVE instructions. This method keeps track if we will be writing into GPR registers and if so, need to remove that register from GC tracking.

Fixes: #112362

@dotnet/arm64-contrib

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 11, 2025
@@ -1087,6 +1087,22 @@ bool emitter::emitInsMayWriteToGCReg(instrDesc* id)
case IF_SR_1A: // SR_1A ................ ...........ttttt Rt (dc zva, mrs)
return ins == INS_mrs_tpid0;

// Below SVE instructions write to GPR and hence GC reg
Copy link
Contributor

Choose a reason for hiding this comment

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

How did you figure out this list? Just want to make sure we've got all of them

Copy link
Member Author

Choose a reason for hiding this comment

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

I just went through the instrsarm64sve.h and went over the instructions that has GPR as destination register and added them to this list.

Copy link
Contributor

@a74nh a74nh left a comment

Choose a reason for hiding this comment

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

LGTM, assuming the list is complete

@kunalspathak
Copy link
Member Author

@dotnet/jit-contrib

@kunalspathak
Copy link
Member Author

/azp run runtime-coreclr gcstress-extra

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kunalspathak
Copy link
Member Author

Sample diff:

image

@kunalspathak
Copy link
Member Author

/azp run runtime-coreclr gcstress0x3-gcstress0xc

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kunalspathak
Copy link
Member Author

I do see some new failures emerged after the original one was passing. I will look little more to make sure all are addressed before merging this PR:

16:36:57.124 Passed test: _Sve_r::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_LoadVectorFirstFaulting_byte()
16:36:58.083 Running test: _Sve_r::JIT.HardwareIntrinsics.Arm._Sve.Program.Sve_LoadVectorInt32SignExtendFirstFaulting_long()
Beginning scenario: RunBasicScenario_Load
App Exit Code: 259
Expected: 100
Actual: 259
END EXECUTION - FAILED
FAILED

@kunalspathak
Copy link
Member Author

alright, so the issue is some kind of timeout that is getting triggered for all the partitions of the test and not anything underlying. I will go ahead and merge this one and follow up on the above mentioned issue.

@kunalspathak kunalspathak merged commit 8c92ae7 into dotnet:main Feb 12, 2025
132 of 144 checks passed
@kunalspathak kunalspathak deleted the sve-gcinfo branch February 12, 2025 00:56
@kunalspathak
Copy link
Member Author

Opened #112463 to track it.

grendello added a commit to grendello/runtime that referenced this pull request Feb 12, 2025
* main:
  [Android] Run CoreCLR functional tests on Android (dotnet#112283)
  [LoongArch64] Fix some assertion failures for Debug ILC building Debug NativeAOT testcases. (dotnet#112229)
  Fix suspicious code fragments (dotnet#112384)
  `__ComObject` doesn't support dynamic interface map (dotnet#112375)
  Native DLLs: only load imported DLLs from System32 (dotnet#112359)
  [main] Update dependencies from dotnet/roslyn (dotnet#112314)
  Update SVE instructions that writes to GC regs (dotnet#112389)
  Bring up android+coreclr windows build.  (dotnet#112256)
  Never use heap for return buffers (dotnet#112060)
  Wait to complete the test before releasing the agile reference. (dotnet#112387)
  Prevent returning disposed HTTP/1.1 connections to the pool (dotnet#112383)
  Fingerprint dotnet.js if writing import map to html is enabled (dotnet#112407)
  Remove duplicate definition of CORECLR_HOSTING_API_LINKAGE (dotnet#112096)
  Update the exception message to reflect current behavior. (dotnet#112355)
  Use enum for frametype not v table (dotnet#112166)
  Enable AltJits build for LoongArch64 and RiscV64 (dotnet#110282)
  Guard members of MonoType union & fix related bugs (dotnet#111645)
  Add optional hooks for debugging OpenSSL memory allocations (dotnet#111539)
  JIT: Optimize struct parameter register accesses in the backend (dotnet#110819)
  NativeAOT: Cover more opcodes in type preinitializer (dotnet#112073)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JIT: HardwareIntrinsics_Arm tests failing with GC holes
3 participants