SPMI for Arm64 failing asserting for argReg == argNode->GetRegNum()

[tannergooding]

Build Information

Build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=643030&view=logs&jobId=80130f07-32fc-5c19-809b-35051dcc5ef5
Build error leg or test failing: SuperPMI replay windows x64 checked

Error Message

Fill the error message using step by step known issues guidance.

{
  "ErrorMessage": "argReg == argNode->GetRegNum()",
  "BuildRetry": false,
  "ExcludeConsoleLog": false
}

Known issue validation

Build: 🔎 https://dev.azure.com/dnceng-public/public/_build/results?buildId=643030
Error message validated: [argReg == argNode->GetRegNum()]
Result validation: ❌ Known issue did not match with the provided build.
Validation performed at: 4/16/2024 2:58:03 PM UTC

Report

Summary

24-Hour Hit Count	7-Day Hit Count	1-Month Count
0	0	0

[dotnet-policy-service]

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

[tannergooding]

This is failing for a number of the legs (any that actually run the SPMI jobs): https://dev.azure.com/dnceng-public/public/_build?definitionId=150&_a=summary

This was likely introduced in #100589 and is probably related to the LoadVector128x4 or related APIs which are relatively new.

[dotnet-policy-service]

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

[AndyAyersMS]

Build analysis seems not to be matching on this... any idea why?

[jeffschwMSFT]

Build analysis seems not to be matching on this... any idea why?

Build analysis has been restricted to the CI/PR pipeline for now. There is engineering work to re-enable for our other pipelines.

[kunalspathak]

@SwapnilGaikwad

[a74nh]

The storex instructions allow for register wraparound (eg 30, 31, 0, 1). If the register allocator supports this, then the assert in hwintrinsiccodegenarm64.cpp is incorrect - it needs to allow for the register to wrap back to 0.

Alternatively - if the register allocator doesn't support wrap around then my only thought is that the allocator is failing to allocate a valid sequence and silently failing.

[kunalspathak]

The storex instructions allow for register wraparound (eg 30, 31, 0, 1). If the register allocator supports this, then the assert in hwintrinsiccodegenarm64.cpp is incorrect - it needs to allow for the register to wrap back to 0.

LSRA does support wrap around, so we should update the assert accordingly. But have you confirmed that this indeed is a wrap around case? During my testing, I never encountered the case where wrap around was needed.

runtime/src/coreclr/jit/lsraarm64.cpp

Lines 242 to 301 in f021ccf

	// Finally, check for round robin case where sequence of last register
	// round to first register is available.
	// For n registers needed, it checks if MSB (n-1) + LSB (1) or
	// MSB (n - 2) + LSB (2) registers are available and if yes,
	// set the least bit of such MSB.
	//
	// This could have done using bit-twiddling, but is simpler when the
	// checks are done with these hardcoded values.
	switch (registersNeeded)
	{
	case 2:
	{
	if ((candidates & v0_v31_mask) != RBM_NONE)
	{
	consecutiveResult |= RBM_V31;
	overallResult |= v0_v31_mask;
	}
	break;
	}
	case 3:
	{
	regMaskTP v0_v30_v31_mask = RBM_V0 | RBM_V30 | RBM_V31;
	if ((candidates & v0_v30_v31_mask) != RBM_NONE)
	{
	consecutiveResult |= RBM_V30;
	overallResult |= v0_v30_v31_mask;
	}
	regMaskTP v0_v1_v31_mask = RBM_V0 | RBM_V1 | RBM_V31;
	if ((candidates & v0_v1_v31_mask) != RBM_NONE)
	{
	consecutiveResult |= RBM_V31;
	overallResult |= v0_v1_v31_mask;
	}
	break;
	}
	case 4:
	{
	regMaskTP v0_v29_v30_v31_mask = RBM_V0 | RBM_V29 | RBM_V30 | RBM_V31;
	if ((candidates & v0_v29_v30_v31_mask) != RBM_NONE)
	{
	consecutiveResult |= RBM_V29;
	overallResult |= v0_v29_v30_v31_mask;
	}
	regMaskTP v0_v1_v30_v31_mask = RBM_V0 | RBM_V29 | RBM_V30 | RBM_V31;
	if ((candidates & v0_v1_v30_v31_mask) != RBM_NONE)
	{
	consecutiveResult |= RBM_V30;
	overallResult |= v0_v1_v30_v31_mask;
	}
	regMaskTP v0_v1_v2_v31_mask = RBM_V0 | RBM_V29 | RBM_V30 | RBM_V31;
	if ((candidates & v0_v1_v2_v31_mask) != RBM_NONE)
	{
	consecutiveResult |= RBM_V31;
	overallResult |= v0_v1_v2_v31_mask;
	}
	break;
	}

[SwapnilGaikwad]

then the assert in hwintrinsiccodegenarm64.cpp is incorrect - it needs to allow for the register to wrap back to 0.

Indeed, we needed to wraparound the argReg - #101430 implements it.

[kunalspathak]

Fixed by #101430