Skip to content

[release/7.0] Allow SIMD-returning calls as arguments - #74520

Merged
carlossanlop merged 2 commits into
release/7.0from
backport/pr-74184-to-release/7.0
Aug 25, 2022
Merged

[release/7.0] Allow SIMD-returning calls as arguments#74520
carlossanlop merged 2 commits into
release/7.0from
backport/pr-74184-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Aug 24, 2022

Copy link
Copy Markdown
Contributor

Backport of #74184 to release/7.0

/cc @BruceForstall@SingleAccretion

Customer Impact

Possible silent bad codegen for code involving SIMD types.

Testing

Normal CI testing including SuperPMI replay/asmdiffs

Risk

low

As of this change we handle all relevant ABI scenarios.
1) Windows x64:
- SIMD8: returned and passed as "TYP_LONG", fine.
- SIMD12 / SIMD16 / SIMD32: returned and passed via a return buffer, fine.
2) Unix x64:
- SIMD8: returned and passed in one FP register, fine.
- SIMD12 / SIMD16, Vector4: returned and passed in two FP registers, fine.
- SIMD16, Vector128 / SIMD32: returned and passed via a return buffer, fine.
3) x86:
- SIMD8: can be returned via two registers or a return buffer (and is always passed on stack), both are fine.
- SIMD12/SIMD16/SIMD32: returned via a return buffer, passed on stack, fine.
4) ARM64:
- SIMD8, Vector2: returned in two FP registers (and passed as such or "TYP_LONG" under Windows varargs), fine.
- SIMD8, Vector64: returned in one FP register, can be passed as such or as "TYP_LONG" under Windows varargs.
The latter case is now handled correctly in "Lowering::LowerArg".
- SIMD12: returned in three FP registers, passed as such or in two integer registers under Windows varargs, fine.
- SIMD16, Vector4: returned in four FP registers, passed as such, or in two integer registers under Windows varargs, fine.
- SIMD16, Vector128: returned in one FP register, passed as such, or in two integer registers under Windows varargs, fine
(morph will decompose the varargs case into a `FIELD_LIST` via a temp).
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 24, 2022
@ghost

Copy link
Copy Markdown

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

Issue Details

Backport of #74184 to release/7.0

/cc @BruceForstall@SingleAccretion

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-CodeGen-coreclr

Milestone:-

@SingleAccretion

Copy link
Copy Markdown
Contributor

Customer Impact
Possible silent bad codegen for code involving SIMD types.

Nit: I think the observable symptom of this is silent bad performance; the optimized code will fix hit an unreached aka noway_assert and then fall back to minopts, which will succeed.

additional justification for porting the fix back to 7.0

This is a regression from 6.0, caused by a combination of forward substitution and OBJ(ADDR(LCL_VAR)) => LCL_VAR folding for call arguments (both new features).

@carlossanlop

Copy link
Copy Markdown
Contributor

Can we please get an approval, @jeffschwMSFT ?

@JulieLeeMSFT

Copy link
Copy Markdown
Member

@BruceForstall I reran the failed tests. Once all tests pass, please merge.

@JulieLeeMSFT

Copy link
Copy Markdown
Member

@carlossanlop all tests are green. It is good to merge.

@carlossanlop
carlossanlop merged commit 2c5d001 into release/7.0Aug 25, 2022
@carlossanlop
carlossanlop deleted the backport/pr-74184-to-release/7.0 branch August 25, 2022 21:30
@ghostghost locked as resolved and limited conversation to collaborators Sep 25, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-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.

4 participants

@SingleAccretion@carlossanlop@JulieLeeMSFT@BruceForstall