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

Allow more obj(addr(lcl_var) foldings. #42343

Merged
merged 2 commits into from
Sep 18, 2020

Conversation

sandreenko
Copy link
Contributor

Diffs on libraries:

x64 win pmi: -18832 (-0.037% of base)
arm64 win pmi: -6532 (-0.012% of base)
x64 unix pmi: -10302 (-0.020% of base)

x64 win crossgen: -1011 (-0.003% of base)
arm64 win crossgen: -352 (-0.001% of base)
x64 unix crossgen: -527 (-0.001% of base)

a different number of improved methods, no regression.

x64 win pmi of System.Net.Http benefits the most:

       -6867 : System.Net.Http.dasm (-0.932% of base)

a typical improvement comes from copy propagation when we can delete an unnecessary block copy:

@@ -1,16 +1,7 @@
-***** BB01
-STMT00003 (IL 0x000...  ???)
-N005 ( 13, 12) [000017] -A------R---              *  ASG       struct (copy)
-N004 (  9,  9) [000016] n-----------              +--*  OBJ       struct<Microsoft.CodeAnalysis.SyntaxTriviaList, 40>
-N003 (  3,  5) [000015] ------------              |  \--*  ADDR      byref
-N002 (  3,  4) [000012] D------N----              |     \--*  LCL_FLD   struct V03 tmp1         d:1[+0] Fseq[_list]
-N001 (  3,  2) [000013] ------------              \--*  LCL_VAR   struct<Microsoft.CodeAnalysis.SyntaxTriviaList, 40> V04 tmp2         u:1 (last use)
-
-***** BB01
 STMT00001 (IL 0x00B...  ???)
 N004 ( 11,  8) [000008] -A-X--------              *  ASG       struct (copy)
 N002 (  7,  5) [000007] D--X--------              +--*  OBJ       struct<Reversed, 40>
 N001 (  1,  1) [000006] ------------              |  \--*  LCL_VAR   byref  V01 RetBuf       u:1
-N003 (  3,  2) [000005] ------------              \--*  LCL_VAR   struct<Reversed, 40> V03 tmp1         u:1 (last use)
+N003 (  3,  2) [000005] ------------              \--*  LCL_VAR   struct<Microsoft.CodeAnalysis.SyntaxTriviaList, 40> V04 tmp2         u:1 (last use)

-Method code size: 136
\ No newline at end of file
+Method code size: 81
\ No newline at end of file

@sandreenko sandreenko added enhancement Product code improvement that does NOT require public API changes/additions area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Sep 16, 2020
@sandreenko
Copy link
Contributor Author

It is a continuation for #42320 in a preparation for arm64 apple changes.
PTAL @CarolEidt @dotnet/jit-contrib

@@ -453,7 +451,10 @@ bool ClassLayout::AreCompatible(const ClassLayout* layout1, const ClassLayout* l
return true;
}

assert(clsHnd1 != NO_CLASS_HANDLE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did the asserts move down? Seems strange to allow clsHnd2 == NO_CLASS_HANDLE for the above checks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could have a situation when we have GT_BLOCK <40>(GT_ADDR ref (LCL_VAR struct<40>)). GenTreeBlock nodes do not have a class handle, they represent a memory chunk. Such chunk is compatible with a struct if they have the same size and the struct does not have GC pointers.

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 - thanks! I'm liking these incremental improvements!

@sandreenko sandreenko merged commit 92b125f into dotnet:master Sep 18, 2020
@sandreenko sandreenko deleted the moreLayoutImprovements branch September 18, 2020 05:22
@briansull
Copy link
Contributor

Regresses libraries testing:

Issue #42517 Test failure: System.Buffers.Tests.ArrayBufferWriterTests_Char.Advance

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants