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

No retyping arm/arm64. #36866

Merged
merged 6 commits into from
Jun 8, 2020
Merged

No retyping arm/arm64. #36866

merged 6 commits into from
Jun 8, 2020

Conversation

sandreenko
Copy link
Contributor

@sandreenko sandreenko commented May 22, 2020

Support compDoOldStructRetyping for arm64 and arm32.
Because of #37341 this requires some special handling in import/morph phases.

No diffs with compDoOldStructRetyping , diffs, when retyping is disabled, are similar to what we saw on x64 and x86 (!compDoOldStructRetyping) around ~0.18% regression because of ASG(LCL_VAR with 1 field, call struct handling.

No test failures (note that I will have to disable the test from #37341 because it is failing with compDoOldStructRetyping), I have triggered jitstress and jitstressregs for the changes.

@sandreenko sandreenko added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 22, 2020
@sandreenko sandreenko force-pushed the noRetypingArm branch 3 times, most recently from ebacf18 to befd9a1 Compare June 3, 2020 00:50
@sandreenko sandreenko marked this pull request as ready for review June 5, 2020 01:29
if (effectiveVal->IsCall())
{
#ifdef DEBUG
GenTreeCall* call = effectiveVal->AsCall();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this block will go away after #11413 is fixed as discussed in #37341

@sandreenko
Copy link
Contributor Author

PTAL @CarolEidt, @dotnet/jit-contrib

Copy link
Contributor

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

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

LGTM with a few comments - thanks for expanding the test!


// Returns true if the method returns a value in more than one return register,
// it should replace/be merged with compMethodReturnsMultiRegRetType when #36868 is fixed.
bool compMethodReturnsResInMultiplyRegisters()
Copy link
Contributor

Choose a reason for hiding this comment

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

Even if temporary, I think it would be good to 1) describe in what way it is different (in particular talk about the special handling required for SIMD types, soon to be additional ones for x64/ux, that are passed in a single register), and 2) rename it to be something like compMethodReturnsMultiRegRegTypeAlternate.

src/coreclr/src/jit/compiler.h Show resolved Hide resolved
src/coreclr/src/jit/importer.cpp Show resolved Hide resolved
src/coreclr/src/jit/lower.cpp Outdated Show resolved Hide resolved
@sandreenko sandreenko merged commit a3bb3c0 into dotnet:master Jun 8, 2020
@sandreenko sandreenko deleted the noRetypingArm branch June 8, 2020 23:45
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
@sandreenko sandreenko restored the noRetypingArm branch September 2, 2021 05:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm32 arch-arm64 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.

2 participants