Skip to content

Fix assert(isByteReg(ireg)). - #46567

Merged
sandreenko merged 1 commit into
dotnet:masterfrom
sandreenko:fixByteRegx86
Jan 6, 2021
Merged

Fix assert(isByteReg(ireg)).#46567
sandreenko merged 1 commit into
dotnet:masterfrom
sandreenko:fixByteRegx86

Conversation

@sandreenko

@sandreenkosandreenko commented Jan 5, 2021

Copy link
Copy Markdown
Contributor

We had a similar code in the non-multi reg case:

#ifdef TARGET_X86
var_types type = varDsc->GetRegisterType(storeLoc);
if (varTypeIsByte(type))
{
srcCandidates = allByteRegs();
}
#endif// TARGET_X86

so repeat the logic for MultiReg.

When we do a call we then copy the arguments to the temp location and we should choose registers that can be used in byte instructions on x86.

Fixes#46277.

No diffs.

@sandreenkosandreenko added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 5, 2021
@sandreenko

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr outerloop, runtime-coreclr jitstressregs

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@sandreenko

Copy link
Copy Markdown
ContributorAuthor

PTAL @dotnet/jit-contrib , cc @jkoritzinsky

@sandreenko

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime-coreclr jitstressregs

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@BruceForstall
BruceForstall self-requested a review January 5, 2021 18:49

@kunalspathakkunalspathak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me. However, I am wondering what changed that we hit issue #46277? I see that the assert in the issue was added last year in 9781175?

@sandreenko

Copy link
Copy Markdown
ContributorAuthor

Looks good to me. However, I am wondering what changed that we hit issue #46277? I see that the assert in the issue was added last year in 9781175?

It was exposed by #39294, before Jit did not see struct returns like <int, byte> on x86.

@sandreenko

Copy link
Copy Markdown
ContributorAuthor

The failures in the stress job are covered by #46085.

@sandreenko
sandreenko merged commit c0ca958 into dotnet:masterJan 6, 2021
@sandreenko
sandreenko deleted the fixByteRegx86 branch January 6, 2021 01:58
@ghostghost locked as resolved and limited conversation to collaborators Feb 5, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-x86area-CodeGen-coreclrCLR 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.

Test failure: Assertion failed 'isByteReg(ireg)' in 'StructABI:EchoByteAndFloatWrapper():bool' during 'Generate code'

3 participants

@sandreenko@BruceForstall@kunalspathak