Fold full-width SIMD-typed indirections #76745

SingleAccretion · 2022-10-07T15:34:20Z

I. e. OBJ/IND<simd>(ADDR(LCL_VAR<simd>)).

Some positive (in nature) diffs, with a TP win as well.

ghost · 2022-10-07T15:34:33Z

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

Issue Details

I. e. OBJ/IND<simd>(ADDR(LCL_VAR<simd>)). We're expecting minor positive diffs.

Author:	SingleAccretion
Assignees:	-
Labels:	`area-CodeGen-coreclr`
Milestone:	-

SingleAccretion · 2022-10-07T18:42:04Z

@dotnet/jit-contrib

We're getting close to completing the "fold all indirect access in local morph" part of the ADDR work.

EgorBo · 2022-10-07T20:52:05Z

positive diffs

doesn't sound good 😄 luckily, they're negative in fact!

EgorBo · 2022-10-07T20:54:28Z

Do you want to run some outerloop here? Since over the last few weeks it was constantly in a bad shape we try to run them in all potentially impactful PRs explicitly

SingleAccretion · 2022-10-07T20:56:26Z

Do you want to run some outerloop here? Since over the last few weeks it was constantly in a bad shape we try to run them in all potentially impactful PRs explicitly

Yeah, seems a good idea ([libraries-]stress would be the pipelines I suppose).

positive diffs

I have a questionable habit of characterizing diffs by their "niceness" 😄.

EgorBo · 2022-10-07T20:57:25Z

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

azure-pipelines · 2022-10-07T20:57:44Z

Azure Pipelines successfully started running 2 pipeline(s).

SingleAccretion · 2022-10-08T12:18:40Z

I looked at the stress failures, all looked pre-existing, except one on ARM:


Assert failure(PID 61 [0x0000003d], Thread: 119 [0x0077]): Assertion failed 'cls != nullptr && !isInvalidHandle(cls)' in 'Analysis:<MakeAndAssignEnvironments>b__12_0(Scope):this' during 'Morph - Inlining' (IL size 241; hash 0x4aab3218; FullOpts)

    File: /__w/1/s/src/coreclr/jit/_typeinfo.h Line: 235
    Image: /root/helix/work/correlation/dotnet

I looked at the dump, and what we have there is a function being inlined, of which the first parameter is a reference type, but when looking at the signature, we get, rather mysteriously, CORINFO_TYPE_VALUECLASS. The method table of this type (as returned by getArgType) looked uninitialized (assuming I got the right stack location for it, I suppose):

0:025> ?? (MethodTable*)0xd0071090
MethodTable * 0xd0071090
   +0x000 m_dwFlags        : 0xf4884050
   +0x004 m_BaseSize       : 0xd0071c90
   +0x008 m_wFlags2        : 0x113c
   +0x00a m_wToken         : 0xd007
   +0x00c m_wNumVirtuals   : 0xdb5c
   +0x00e m_wNumInterfaces : 0xe1d6
   +0x010 debug_m_szClassName : (null) 
   +0x014 m_pParentMethodTable : (null) 
   +0x018 m_pLoaderModule  : 0xffffffff Module
   +0x01c m_pWriteableData : 0x12340000 MethodTableWriteableData
   +0x020 m_pEEClass       : (null) 
   +0x020 m_pCanonMT       : 0
   +0x024 m_pPerInstInfo   : (null) 
   +0x024 m_ElementTypeHnd : 0
   +0x024 m_pMultipurposeSlot1 : 0
   +0x028 m_pInterfaceMap  : 0xdfc265b0 InterfaceInfo_t
   +0x028 m_pMultipurposeSlot2 : 0xdfc265b0

Will assume not related as there are no SIMD types on ARM.

Will also hold off on resolving the conflicts until the CI is up.

We should really delete the SIMD "InitObj" IR form, but that change had some unfortunate regressions to it, so, for now, just make it work reliably.

SingleAccretion · 2022-10-25T06:13:12Z

Looks like there are a couple SPMI failures that will need to be investigated.

Consider a promoted two-field HFA stack argument on ARM64. This argument needs to be processed by multi-reg morphing, so that it is transformed into a FIELD_LIST, otherwise it would end up independently promoted yet appear in the IR as a LCL_VAR. That was not happening because the number of stack slots consumed by such an argument is 1 (8 bytes), and the number of registers is zero (naturally). As the fix, use the more obviously correct check based on "structBaseType" to detect when we need to invoke multi-reg morphing. No diffs.

SingleAccretion · 2022-10-27T18:01:05Z

Looks like there are a couple SPMI failures that will need to be investigated.

The issues have been solved and this is once again ready for review.

jakobbotsch · 2022-10-31T10:04:07Z

src/coreclr/jit/morph.cpp

+#if !defined(TARGET_X86)
+                else
+                {
+                    hasMultiregStructArgs = true;


Why is this needed here now?

It used to be set below:

#if FEATURE_MULTIREG_ARGS if (isStructArg) { if (((arg.AbiInfo.NumRegs + arg.AbiInfo.GetStackSlotsNumber()) > 1) || (isHfaArg && argx->TypeGet() == TYP_STRUCT)) { hasMultiregStructArgs = true; } }

So this is just moving it to here. The benefit of this location is that we don't need to invoke multi-reg morphing if we have already transformed everything into FIELD_LISTs.

jakobbotsch · 2022-10-31T10:05:57Z

src/coreclr/jit/morph.cpp

            call->gtArgs.SetTemp(&arg, tmp);
-            lvaSetVarAddrExposed(tmp DEBUGARG(AddressExposedReason::TOO_CONSERVATIVE));
+            hasMultiregStructArgs |= ((arg.AbiInfo.ArgType == TYP_STRUCT) && !arg.AbiInfo.PassedByRef);


I assume this was a preexisting issue on platforms with by-value struct args?

No, as with https://github.com/dotnet/runtime/pull/76745/files#r1009281151, this was just moved.

jakobbotsch

LGTM, nice clean up too.

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 7, 2022

ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 7, 2022

SingleAccretion mentioned this pull request Oct 7, 2022

Remove GT_ADDR nodes #11057

Closed

SingleAccretion force-pushed the LclMorph-SimdLclVar-Upstream branch from dcca3cd to 22e9c1c Compare October 7, 2022 16:22

SingleAccretion marked this pull request as ready for review October 7, 2022 18:39

SingleAccretion mentioned this pull request Oct 7, 2022

Fold primitive-typed access to promoted structs in local morph and forbid mismatched struct assignments #76766

Merged

build-analysis bot mentioned this pull request Oct 8, 2022

503's dotnet/arcade#10943

Closed

2 tasks

SingleAccretion added 3 commits October 10, 2022 14:12

Fold simple SIMD indirs in local morph

af06641

Fix various downstream bugs

e492874

We should really delete the SIMD "InitObj" IR form, but that change had some unfortunate regressions to it, so, for now, just make it work reliably.

Delete code

f288eeb

SingleAccretion force-pushed the LclMorph-SimdLclVar-Upstream branch from 22e9c1c to f288eeb Compare October 10, 2022 11:13

SingleAccretion mentioned this pull request Oct 10, 2022

Split and simplify fgMorphOneAsgBlockOp #76793

Merged

SingleAccretion closed this Oct 24, 2022

SingleAccretion reopened this Oct 24, 2022

SingleAccretion force-pushed the LclMorph-SimdLclVar-Upstream branch from ed45b7a to f289c26 Compare October 25, 2022 20:49

This was referenced Oct 25, 2022

Tracking Nuget 429s dotnet/arcade#10885

Closed

Tracking issue for CI build timeouts #76454

Closed

This was referenced Oct 25, 2022

Infra improvements for Helix #68176

Closed

Long Running Test: Interop/MonoAPI/MonoMono/PInvokeDetach/PInvokeDetach.sh #73040

Closed

SingleAccretion force-pushed the LclMorph-SimdLclVar-Upstream branch from f289c26 to 1394fc0 Compare October 26, 2022 17:34

SingleAccretion closed this Oct 26, 2022

SingleAccretion reopened this Oct 26, 2022

SingleAccretion closed this Oct 27, 2022

SingleAccretion reopened this Oct 27, 2022

jakobbotsch reviewed Oct 31, 2022

View reviewed changes

jakobbotsch approved these changes Oct 31, 2022

View reviewed changes

jakobbotsch merged commit 35cb312 into dotnet:main Oct 31, 2022

SingleAccretion deleted the LclMorph-SimdLclVar-Upstream branch October 31, 2022 15:11

ghost locked as resolved and limited conversation to collaborators Nov 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fold full-width SIMD-typed indirections #76745

Fold full-width SIMD-typed indirections #76745

SingleAccretion commented Oct 7, 2022 •

edited

Loading

ghost commented Oct 7, 2022

SingleAccretion commented Oct 7, 2022

EgorBo commented Oct 7, 2022

EgorBo commented Oct 7, 2022

SingleAccretion commented Oct 7, 2022

EgorBo commented Oct 7, 2022

azure-pipelines bot commented Oct 7, 2022

SingleAccretion commented Oct 8, 2022 •

edited

Loading

SingleAccretion commented Oct 25, 2022

SingleAccretion commented Oct 27, 2022

jakobbotsch Oct 31, 2022

SingleAccretion Oct 31, 2022 •

edited

Loading

jakobbotsch Oct 31, 2022

SingleAccretion Oct 31, 2022

jakobbotsch left a comment

Fold full-width SIMD-typed indirections #76745

Fold full-width SIMD-typed indirections #76745

Conversation

SingleAccretion commented Oct 7, 2022 • edited Loading

ghost commented Oct 7, 2022

SingleAccretion commented Oct 7, 2022

EgorBo commented Oct 7, 2022

EgorBo commented Oct 7, 2022

SingleAccretion commented Oct 7, 2022

EgorBo commented Oct 7, 2022

azure-pipelines bot commented Oct 7, 2022

SingleAccretion commented Oct 8, 2022 • edited Loading

SingleAccretion commented Oct 25, 2022

SingleAccretion commented Oct 27, 2022

jakobbotsch Oct 31, 2022

Choose a reason for hiding this comment

SingleAccretion Oct 31, 2022 • edited Loading

Choose a reason for hiding this comment

jakobbotsch Oct 31, 2022

Choose a reason for hiding this comment

SingleAccretion Oct 31, 2022

Choose a reason for hiding this comment

jakobbotsch left a comment

Choose a reason for hiding this comment

SingleAccretion commented Oct 7, 2022 •

edited

Loading

SingleAccretion commented Oct 8, 2022 •

edited

Loading

SingleAccretion Oct 31, 2022 •

edited

Loading