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.

Sign up for GitHub

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

Jump to bottom

Better codegen for (T)x | (T)y #58727

Merged
merged 8 commits into from
Oct 12, 2021
Merged

Better codegen for (T)x | (T)y #58727

merged 8 commits into from
Oct 12, 2021

Conversation

benjamin-hodgson
Copy link
Contributor

@benjamin-hodgson benjamin-hodgson commented Sep 6, 2021 •
edited
Loading

I added a morph pass to fold expressions like (T)x | (T)y into (T)(x | y). This results in fewer movzx instructions in the asm.

This is my first contribution to the JIT and I am still learning the code, so any feedback you have for me would be much appreciated! In particular, I'm not certain I got the code to manipulate the GenTree exactly right - I was following other examples in the codebase but I may have missed something about the GenTree API.

I added my morph to the bottom-up ("post-order") passes in fgMorphSmpOp, right after the morph which folds bitwise rotations. If you think this isn't an appropriate place for this optimisation please let me know.

The last example from the original issue (DowncastPtr) doesn't seem to work yet - it seems the JIT doesn't recognise *(T*)&x == (T)x (when the cast is a downcast). That seems to me like a separate optimisation; I can file an issue and/or fix it if you like.

I added a test in CodeGenBringUpTests to make sure this branch was being exercised, but I'm not sure whether that's the right place for it. I couldn't find any lower-level tests which look at GenTrees directly, I assumed that was a deliberate testing policy choice but would be happy to add one if I missed it.

Tested on my 64-bit Intel MacBook.

Fixes #13816

Better codegen for (T)x | (T)y
80f398d 
I added a morph pass to fold expressions like `(T)x | (T)y`
into `(T)(x | y)`. This results in fewer `movzx` instructions
in the asm.

Fixes #13816
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 6, 2021
@benjamin-hodgson benjamin-hodgson mentioned this pull request Sep 6, 2021
Closed
EgorBo
EgorBo reviewed Sep 6, 2021
View reviewed changes
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
EgorBo
EgorBo reviewed Sep 6, 2021
View reviewed changes
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
EgorBo
EgorBo reviewed Sep 6, 2021
View reviewed changes
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
SingleAccretion
SingleAccretion reviewed Sep 6, 2021
View reviewed changes
Copy link
Contributor

@SingleAccretion SingleAccretion left a comment

Choose a reason for hiding this comment

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

Just nits from me, Egor's noted the substantial correctness issues already.

Codegen diffs for this change will be asked for, one of the easier ways to get them is with the SPMI tool.

I also note that there are more variants of this optimizations possible, since, e. g. AND with a zero-extending cast makes the other cast effectively zero-extending as well. Seems unlikely to be that valuable to catch them all here, however.

src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
@benjamin-hodgson
Copy link
Contributor Author

Thank you very much for the feedback! I will fix up the code with your suggestions this afternoon.

Benjamin Hodgson added 5 commits September 6, 2021 17:18
Code review updates
cdec019 
* Rename function to fgMorphCastedBitwiseOp
* Don't fold checked arithmetic
* Reuse op1 node for return value
* Don't run outside global morphing
* Various code style and comment tweaks
Don't call gtGetOp2 if tree was folded.
838b35b 
If it was folded, it was folded to a unary (cast) operation
and gtGetOp2() will crash.

I also tweaked fgMorphCastedBitwiseOp to return nullptr
if it didn't do anything (to match behaviour of fgMorphCommutative)
Code review changes for tests
acf28b8 
* Removed all but one csproj
* Added tests for scenarios with overflow, compound exprs, side effects
Add in some asserts and remove a redundant call
fa787fc
fix typo
ae02508
@runfoapp runfoapp bot mentioned this pull request Sep 7, 2021
Closed
SingleAccretion
SingleAccretion approved these changes Sep 7, 2021
View reviewed changes
Copy link
Contributor

@SingleAccretion SingleAccretion left a comment

Choose a reason for hiding this comment

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

LGTM, modulo suggestions. You will also need to fix the formatting.

src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/morph.cpp Outdated Show resolved Hide resolved
Benjamin Hodgson added 2 commits September 11, 2021 10:33
Code review changes:
c32cb0f 
* Formatting
* Use getters instead of fields
* set flags on op1
Fix formatting
0b59928
@JulieLeeMSFT
Copy link
Member

@benjamin-hodgson Thanks for your first contribution.
@EgorBo PTAL the community PR.

@JulieLeeMSFT JulieLeeMSFT added this to the 7.0.0 milestone Oct 4, 2021
@EgorBo
Copy link
Member

EgorBo commented Oct 11, 2021

Sorry for the delay, looks good to me

@EgorBo
Copy link
Member

EgorBo commented Oct 12, 2021

aspnet.run.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 11960181 (overridden on cmd)
Total bytes of diff: 11960127 (overridden on cmd)
Total bytes of delta: -54 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs 


Top file improvements (bytes):
          -8 : 8382.dasm (-2.34% of base)
          -7 : 9254.dasm (-10.94% of base)
          -7 : 3728.dasm (-0.99% of base)
          -6 : 7289.dasm (-0.50% of base)
          -6 : 10724.dasm (-0.39% of base)
          -5 : 8794.dasm (-3.45% of base)
          -5 : 17242.dasm (-0.41% of base)
          -5 : 9084.dasm (-0.26% of base)
          -5 : 8785.dasm (-3.33% of base)

9 total files with Code Size differences (9 improved, 0 regressed), 0 unchanged.

Top method improvements (bytes):
          -8 (-2.34% of base) : 8382.dasm - ConsoleLoggerProvider:DoesConsoleSupportAnsi():bool
          -7 (-10.94% of base) : 9254.dasm - FileSystemInfo:get_LengthCore():long:this
          -7 (-0.99% of base) : 3728.dasm - SocketAddress:GetIPAddress():IPAddress:this
          -6 (-0.50% of base) : 7289.dasm - HostingApplicationDiagnostics:StartActivity(HttpContext,bool,bool,byref):Activity:this
          -6 (-0.39% of base) : 10724.dasm - HostingApplicationDiagnostics:StartActivity(HttpContext,bool,bool,byref):Activity:this
          -5 (-3.33% of base) : 8785.dasm - ConsolePal:set_BackgroundColor(int)
          -5 (-3.45% of base) : 8794.dasm - ConsolePal:set_ForegroundColor(int)
          -5 (-0.41% of base) : 17242.dasm - DataTable:SetLocaleValue(CultureInfo,bool,bool):bool:this
          -5 (-0.26% of base) : 9084.dasm - HostingApplicationDiagnostics:StartActivity(HttpContext,bool,bool,byref):Activity:this

Top method improvements (percentages):
          -7 (-10.94% of base) : 9254.dasm - FileSystemInfo:get_LengthCore():long:this
          -5 (-3.45% of base) : 8794.dasm - ConsolePal:set_ForegroundColor(int)
          -5 (-3.33% of base) : 8785.dasm - ConsolePal:set_BackgroundColor(int)
          -8 (-2.34% of base) : 8382.dasm - ConsoleLoggerProvider:DoesConsoleSupportAnsi():bool
          -7 (-0.99% of base) : 3728.dasm - SocketAddress:GetIPAddress():IPAddress:this
          -6 (-0.50% of base) : 7289.dasm - HostingApplicationDiagnostics:StartActivity(HttpContext,bool,bool,byref):Activity:this
          -5 (-0.41% of base) : 17242.dasm - DataTable:SetLocaleValue(CultureInfo,bool,bool):bool:this
          -6 (-0.39% of base) : 10724.dasm - HostingApplicationDiagnostics:StartActivity(HttpContext,bool,bool,byref):Activity:this
          -5 (-0.26% of base) : 9084.dasm - HostingApplicationDiagnostics:StartActivity(HttpContext,bool,bool,byref):Activity:this

9 total methods with Code Size differences (9 improved, 0 regressed), 0 unchanged.

coreclr_tests.pmi.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 124779987 (overridden on cmd)
Total bytes of diff: 124779719 (overridden on cmd)
Total bytes of delta: -268 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs 


Top file improvements (bytes):
         -26 : 192088.dasm (-2.47% of base)
         -10 : 9406.dasm (-17.86% of base)
         -10 : 9404.dasm (-17.86% of base)
         -10 : 9400.dasm (-17.86% of base)
          -8 : 1404.dasm (-53.33% of base)
          -8 : 9361.dasm (-15.09% of base)
          -8 : 9349.dasm (-15.09% of base)
          -8 : 1408.dasm (-53.33% of base)
          -8 : 9407.dasm (-15.09% of base)
          -8 : 9347.dasm (-15.09% of base)
          -8 : 9363.dasm (-15.09% of base)
          -8 : 9357.dasm (-15.09% of base)
          -8 : 1410.dasm (-53.33% of base)
          -7 : 9356.dasm (-14.00% of base)
          -7 : 9354.dasm (-14.00% of base)
          -7 : 9350.dasm (-14.00% of base)
          -6 : 1351.dasm (-50.00% of base)
          -6 : 1293.dasm (-23.08% of base)
          -6 : 1411.dasm (-50.00% of base)
          -6 : 1367.dasm (-50.00% of base)

44 total files with Code Size differences (44 improved, 0 regressed), 1 unchanged.

Top method improvements (bytes):
         -26 (-2.47% of base) : 192088.dasm - EMFloat:AddSubInternalFPF(ubyte,byref,byref,byref)
         -10 (-17.86% of base) : 9400.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(byte,byte):byte
         -10 (-17.86% of base) : 9404.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Or(byte,byte):byte
         -10 (-17.86% of base) : 9406.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Xor(byte,byte):byte
          -8 (-53.33% of base) : 1404.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(byte,byte):byte
          -8 (-15.09% of base) : 9407.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(ubyte,ubyte):ubyte
          -8 (-15.09% of base) : 9357.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(ushort,ushort):ushort
          -8 (-53.33% of base) : 1408.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Or(byte,byte):byte
          -8 (-15.09% of base) : 9347.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Or(ubyte,ubyte):ubyte
          -8 (-15.09% of base) : 9361.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Or(ushort,ushort):ushort
          -8 (-53.33% of base) : 1410.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Xor(byte,byte):byte
          -8 (-15.09% of base) : 9349.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Xor(ubyte,ubyte):ubyte
          -8 (-15.09% of base) : 9363.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Xor(ushort,ushort):ushort
          -7 (-14.00% of base) : 9350.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(short,short):short
          -7 (-14.00% of base) : 9354.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Or(short,short):short
          -7 (-14.00% of base) : 9356.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Xor(short,short):short
          -6 (-50.00% of base) : 1411.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(ubyte,ubyte):ubyte
          -6 (-50.00% of base) : 1361.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(ushort,ushort):ushort
          -6 (-5.13% of base) : 9283.dasm - JIT.HardwareIntrinsics.Arm.Helpers:CompareTest(byte,byte):byte
          -6 (-5.41% of base) : 9295.dasm - JIT.HardwareIntrinsics.Arm.Helpers:CompareTest(short,short):short

Top method improvements (percentages):
          -8 (-53.33% of base) : 1404.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(byte,byte):byte
          -8 (-53.33% of base) : 1408.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Or(byte,byte):byte
          -8 (-53.33% of base) : 1410.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Xor(byte,byte):byte
          -6 (-50.00% of base) : 1411.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(ubyte,ubyte):ubyte
          -6 (-50.00% of base) : 1361.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(ushort,ushort):ushort
          -6 (-50.00% of base) : 1351.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Or(ubyte,ubyte):ubyte
          -6 (-50.00% of base) : 1365.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Or(ushort,ushort):ushort
          -6 (-50.00% of base) : 1353.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Xor(ubyte,ubyte):ubyte
          -6 (-50.00% of base) : 1367.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Xor(ushort,ushort):ushort
          -5 (-41.67% of base) : 1354.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(short,short):short
          -5 (-41.67% of base) : 1358.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Or(short,short):short
          -5 (-41.67% of base) : 1360.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Xor(short,short):short
          -3 (-33.33% of base) : 238183.dasm - InlineBool.Program:Id28(bool):bool
          -3 (-33.33% of base) : 238184.dasm - InlineBool.Program:Id30(bool):bool
          -6 (-23.08% of base) : 1293.dasm - JIT.HardwareIntrinsics.Arm.Helpers:CompareTest(ubyte,ubyte):ubyte
          -3 (-20.00% of base) : 238185.dasm - InlineBool.Program:Id31(bool):bool
          -3 (-18.75% of base) : 238186.dasm - InlineBool.Program:Id33(bool):bool
          -3 (-18.75% of base) : 238187.dasm - InlineBool.Program:Id34(bool):bool
         -10 (-17.86% of base) : 9400.dasm - JIT.HardwareIntrinsics.Arm.Helpers:And(byte,byte):byte
         -10 (-17.86% of base) : 9404.dasm - JIT.HardwareIntrinsics.Arm.Helpers:Or(byte,byte):byte

44 total methods with Code Size differences (44 improved, 0 regressed), 1 unchanged.

libraries.crossgen2.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 34847973 (overridden on cmd)
Total bytes of diff: 34847597 (overridden on cmd)
Total bytes of delta: -376 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs 


Top file regressions (bytes):
           8 : 8058.dasm (2.23% of base)
           8 : 98039.dasm (6.45% of base)
           7 : 98035.dasm (9.59% of base)
           5 : 98037.dasm (4.03% of base)
           1 : 30016.dasm (1.96% of base)

Top file improvements (bytes):
         -18 : 178834.dasm (-0.82% of base)
         -11 : 25327.dasm (-0.60% of base)
          -9 : 208301.dasm (-1.15% of base)
          -9 : 159655.dasm (-42.86% of base)
          -8 : 25418.dasm (-15.09% of base)
          -8 : 63384.dasm (-11.59% of base)
          -8 : 96771.dasm (-53.33% of base)
          -8 : 176613.dasm (-3.15% of base)
          -8 : 25406.dasm (-15.09% of base)
          -8 : 25426.dasm (-15.09% of base)
          -8 : 25408.dasm (-15.09% of base)
          -8 : 25428.dasm (-15.09% of base)
          -8 : 138446.dasm (-4.04% of base)
          -8 : 118499.dasm (-12.70% of base)
          -8 : 25416.dasm (-15.09% of base)
          -8 : 96772.dasm (-53.33% of base)
          -8 : 3165.dasm (-12.70% of base)
          -8 : 96770.dasm (-53.33% of base)
          -7 : 25409.dasm (-19.44% of base)
          -7 : 25419.dasm (-19.44% of base)

70 total files with Code Size differences (65 improved, 5 regressed), 1 unchanged.

Top method regressions (bytes):
           8 ( 2.23% of base) : 8058.dasm - Microsoft.CodeAnalysis.CSharp.Symbols.AnonymousTypeManager:IsSameType(Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol,Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol,bool,bool):bool
           8 ( 6.45% of base) : 98039.dasm - System.Half:op_LessThan(System.Half,System.Half):bool
           7 ( 9.59% of base) : 98035.dasm - System.Half:op_Equality(System.Half,System.Half):bool
           5 ( 4.03% of base) : 98037.dasm - System.Half:op_LessThanOrEqual(System.Half,System.Half):bool
           1 ( 1.96% of base) : 30016.dasm - System.Xml.UniqueId:UnsafeDecode(long,ushort,ushort):int:this

Top method improvements (bytes):
         -18 (-0.82% of base) : 178834.dasm - System.Security.Principal.IdentityReferenceCollection:Translate(System.Type,bool):System.Security.Principal.IdentityReferenceCollection:this
         -11 (-0.60% of base) : 25327.dasm - Microsoft.VisualBasic.CompilerServices.OverloadResolution:InsertIfMethodAvailable(System.Reflection.MemberInfo,System.Reflection.ParameterInfo[],int,bool,System.Object[],int,System.String[],System.Type[],bool,System.Collections.Generic.List`1[Microsoft.VisualBasic.CompilerServices.Symbols+Method],Microsoft.VisualBasic.CompilerServices.Symbols+Container)
          -9 (-42.86% of base) : 159655.dasm - System.Reflection.Metadata.SignatureHeader:.ctor(ubyte,ubyte,ubyte):this
          -9 (-1.15% of base) : 208301.dasm - System.UriHelper:EscapeString(System.String,bool,System.ReadOnlySpan`1[System.Boolean],ushort,ushort):System.String
          -8 (-12.70% of base) : 118499.dasm - BaseListEditor:.ctor(Microsoft.CodeAnalysis.Text.TextSpan,int,bool,bool):this
          -8 (-12.70% of base) : 3165.dasm - BaseListEditor:.ctor(Microsoft.CodeAnalysis.Text.TextSpan,int,bool,bool):this
          -8 (-11.59% of base) : 63384.dasm - Microsoft.CodeAnalysis.DesktopAssemblyIdentityComparer:IsOptionallyRetargetableAssembly(Microsoft.CodeAnalysis.AssemblyIdentity):bool
          -8 (-4.04% of base) : 138446.dasm - Microsoft.CodeAnalysis.VisualBasic.Symbols.QuickAttributeChecker:AddName(System.String,ubyte):this
          -8 (-15.09% of base) : 25426.dasm - Microsoft.VisualBasic.CompilerServices.Operators:AndInt16(short,short,System.Type):System.Object
          -8 (-15.09% of base) : 25428.dasm - Microsoft.VisualBasic.CompilerServices.Operators:AndSByte(byte,byte,System.Type):System.Object
          -8 (-15.09% of base) : 25416.dasm - Microsoft.VisualBasic.CompilerServices.Operators:OrInt16(short,short,System.Type):System.Object
          -8 (-15.09% of base) : 25418.dasm - Microsoft.VisualBasic.CompilerServices.Operators:OrSByte(byte,byte,System.Type):System.Object
          -8 (-15.09% of base) : 25406.dasm - Microsoft.VisualBasic.CompilerServices.Operators:XorInt16(short,short,System.Type):System.Object
          -8 (-15.09% of base) : 25408.dasm - Microsoft.VisualBasic.CompilerServices.Operators:XorSByte(byte,byte,System.Type):System.Object
          -8 (-3.15% of base) : 176613.dasm - System.DirectoryServices.AccountManagement.ADStoreCtx:CannotChangePwdFromLdapConverter(System.DirectoryServices.DirectoryEntry):bool
          -8 (-53.33% of base) : 96772.dasm - System.SByte:System.IBitwiseOperators<System.SByte,System.SByte,System.SByte>.op_BitwiseAnd(byte,byte):byte
          -8 (-53.33% of base) : 96771.dasm - System.SByte:System.IBitwiseOperators<System.SByte,System.SByte,System.SByte>.op_BitwiseOr(byte,byte):byte
          -8 (-53.33% of base) : 96770.dasm - System.SByte:System.IBitwiseOperators<System.SByte,System.SByte,System.SByte>.op_ExclusiveOr(byte,byte):byte
          -7 (-19.44% of base) : 25429.dasm - Microsoft.VisualBasic.CompilerServices.Operators:AndBoolean(bool,bool):System.Object
          -7 (-19.44% of base) : 25419.dasm - Microsoft.VisualBasic.CompilerServices.Operators:OrBoolean(bool,bool):System.Object

Top method regressions (percentages):
           7 ( 9.59% of base) : 98035.dasm - System.Half:op_Equality(System.Half,System.Half):bool
           8 ( 6.45% of base) : 98039.dasm - System.Half:op_LessThan(System.Half,System.Half):bool
           5 ( 4.03% of base) : 98037.dasm - System.Half:op_LessThanOrEqual(System.Half,System.Half):bool
           8 ( 2.23% of base) : 8058.dasm - Microsoft.CodeAnalysis.CSharp.Symbols.AnonymousTypeManager:IsSameType(Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol,Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol,bool,bool):bool
           1 ( 1.96% of base) : 30016.dasm - System.Xml.UniqueId:UnsafeDecode(long,ushort,ushort):int:this

Top method improvements (percentages):
          -8 (-53.33% of base) : 96772.dasm - System.SByte:System.IBitwiseOperators<System.SByte,System.SByte,System.SByte>.op_BitwiseAnd(byte,byte):byte
          -8 (-53.33% of base) : 96771.dasm - System.SByte:System.IBitwiseOperators<System.SByte,System.SByte,System.SByte>.op_BitwiseOr(byte,byte):byte
          -8 (-53.33% of base) : 96770.dasm - System.SByte:System.IBitwiseOperators<System.SByte,System.SByte,System.SByte>.op_ExclusiveOr(byte,byte):byte
          -6 (-50.00% of base) : 99328.dasm - System.Byte:System.IBitwiseOperators<System.Byte,System.Byte,System.Byte>.op_BitwiseAnd(ubyte,ubyte):ubyte
          -6 (-50.00% of base) : 99327.dasm - System.Byte:System.IBitwiseOperators<System.Byte,System.Byte,System.Byte>.op_BitwiseOr(ubyte,ubyte):ubyte
          -6 (-50.00% of base) : 99326.dasm - System.Byte:System.IBitwiseOperators<System.Byte,System.Byte,System.Byte>.op_ExclusiveOr(ubyte,ubyte):ubyte
          -6 (-50.00% of base) : 99200.dasm - System.Char:System.IBitwiseOperators<System.Char,System.Char,System.Char>.op_BitwiseAnd(ushort,ushort):ushort
          -6 (-50.00% of base) : 99199.dasm - System.Char:System.IBitwiseOperators<System.Char,System.Char,System.Char>.op_BitwiseOr(ushort,ushort):ushort
          -6 (-50.00% of base) : 99198.dasm - System.Char:System.IBitwiseOperators<System.Char,System.Char,System.Char>.op_ExclusiveOr(ushort,ushort):ushort
          -6 (-50.00% of base) : 94693.dasm - System.UInt16:System.IBitwiseOperators<System.UInt16,System.UInt16,System.UInt16>.op_BitwiseAnd(ushort,ushort):ushort
          -6 (-50.00% of base) : 94692.dasm - System.UInt16:System.IBitwiseOperators<System.UInt16,System.UInt16,System.UInt16>.op_BitwiseOr(ushort,ushort):ushort
          -6 (-50.00% of base) : 94691.dasm - System.UInt16:System.IBitwiseOperators<System.UInt16,System.UInt16,System.UInt16>.op_ExclusiveOr(ushort,ushort):ushort
          -9 (-42.86% of base) : 159655.dasm - System.Reflection.Metadata.SignatureHeader:.ctor(ubyte,ubyte,ubyte):this
          -5 (-41.67% of base) : 97809.dasm - System.Int16:System.IBitwiseOperators<System.Int16,System.Int16,System.Int16>.op_BitwiseAnd(short,short):short
          -5 (-41.67% of base) : 97808.dasm - System.Int16:System.IBitwiseOperators<System.Int16,System.Int16,System.Int16>.op_BitwiseOr(short,short):short
          -5 (-41.67% of base) : 97807.dasm - System.Int16:System.IBitwiseOperators<System.Int16,System.Int16,System.Int16>.op_ExclusiveOr(short,short):short
          -6 (-30.00% of base) : 63958.dasm - Microsoft.CodeAnalysis.RealParser:ShouldRoundUp(bool,bool,bool):bool
          -7 (-19.44% of base) : 25429.dasm - Microsoft.VisualBasic.CompilerServices.Operators:AndBoolean(bool,bool):System.Object
          -7 (-19.44% of base) : 25419.dasm - Microsoft.VisualBasic.CompilerServices.Operators:OrBoolean(bool,bool):System.Object
          -7 (-19.44% of base) : 25409.dasm - Microsoft.VisualBasic.CompilerServices.Operators:XorBoolean(bool,bool):System.Object

70 total methods with Code Size differences (65 improved, 5 regressed), 1 unchanged.

@EgorBo
Copy link
Member

EgorBo commented Oct 12, 2021

A few small regressions due to CSE, can be fixed if we enable this OPT post GlobalMorph I guess, but not sure it's worth it.

Thanks, @benjamin-hodgson!

@EgorBo EgorBo merged commit 68042e1 into dotnet:main Oct 12, 2021
@clamp03 clamp03 mentioned this pull request Oct 13, 2021
Merged
@mangod9
Copy link
Member

mangod9 commented Oct 13, 2021 •
edited
Loading

This is causing a build break with gcc 11: https://dev.azure.com/dnceng/public/_build/results?buildId=1416610&view=logs&j=9e42d44c-0ead-5638-7035-5fe3dab45232&t=a04f53ad-2969-5223-5463-9bb65c1630a9&l=2110

Simple fix is here: #60333

@benjamin-hodgson benjamin-hodgson deleted the issue-13816 branch October 13, 2021 16:45
@ghost ghost locked as resolved and limited conversation to collaborators Nov 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@SingleAccretion SingleAccretion SingleAccretion approved these changes

@EgorBo EgorBo EgorBo approved these changes

Assignees

@EgorBo EgorBo

@benjamin-hodgson benjamin-hodgson

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

JIT: optimize (T)x | (T)y to (T)(x | y)
5 participants
@benjamin-hodgson @JulieLeeMSFT @EgorBo @mangod9 @SingleAccretion