"==0" optimization in Boolean logic #13573 by JulieLeeMSFT · Pull Request #49548 · dotnet/runtime

JulieLeeMSFT · 2021-03-12T19:39:38Z

It optimizes Integral type Boolean logic with two operands when the first BB is conditional BB (e.g., BBJ_COND) and the second BB is conditional return (e.g., GT_RETURN > BBJ_RETURN).
For example, it changes below logic:

x==0 && y==0 to (x|y)==0

In case the folding involves ANDing or comparison to GT_NE, it gets optimized only if the tree is evaluated as BOOLEAN.

x!=0 && y!=0 to (x&y) !=0
x==0 || y==0 to (x&y)==0
x==1 || y==1 to (x|y) !=0

Edit: Note: However, above 3 cases are not optimized because the operand of tree is not evaluated as boolean expression in the current JIT.

This PR does not optimze cases where it requries NOT transformation of x or y because the cost of NOT operation is large.

(x | !y) != 0
(!x | y) != 0

public static bool AreZero(int x, int y) {
return x == 0 && y == 0;
}

3 Basic Blocks are folded as below:
Before:

*** marking local variables in block BB01 (weight=1 )
STMT00000 (IL 0x000...0x001)
[000003] -----+------ * JTRUE void [000002] J----+-N---- \--* NE int [000000] -----+------ +--* LCL_VAR int V00 arg0 [000001] -----+------ \--* CNS_INT int 0
New refCnts for V00: refCnt = 1, refCntWtd = 1 *** marking local variables in block BB02 (weight=0.50)
STMT00002 (IL 0x003...0x007)
[000009] -----+------ * RETURN int [000008] -----+------ \--* EQ int [000006] -----+------ +--* LCL_VAR int V01 arg1 [000007] -----+------ \--* CNS_INT int 0
New refCnts for V01: refCnt = 1, refCntWtd = 0.50
*** marking local variables in block BB03 (weight=0.50)
STMT00001 (IL 0x008...0x009)
[000005] -----+------ * RETURN int [000004] -----+------ \--* CNS_INT int 0

After:

Folded boolean conditions of BB01, BB02 and BB03 to :
STMT00000 (IL 0x000...0x001)
[000003] -----+------ * RETURN int
[000002] J----+-N---- \--* EQ int [000011] ------------ +--* OR int [000000] -----+------ | +--* LCL_VAR int V00 arg0 ( 3, 2) [000006] ------------ | \--* LCL_VAR int V01 arg1 [000001] -----+------ \--* CNS_INT int 0

Diff:

Summary of Code Size diffs:
(Lower is better)
Total bytes of base: 531
Total bytes of diff: 483
Total bytes of delta: -48 (-9.04% of base)
diff is an improvement.
Top file improvements (bytes):
-16 : 113888.dasm (-7.14% of base)
-16 : 159193.dasm (-11.35% of base)
-16 : 109061.dasm (-9.64% of base)
3 total files with Code Size differences (3 improved, 0 regressed), 0 unchanged.
Top method improvements (bytes):
-16 (-7.14% of base) : 113888.dasm - Microsoft.Extensions.Primitives.StringSegment:Equals(Microsoft.Extensions.Primitives.StringSegment,int):bool:this
-16 (-11.35% of base) : 159193.dasm - System.Xml.Linq.XElement:AttributesEqual(System.Xml.Linq.XElement):bool:this
-16 (-9.64% of base) : 109061.dasm - Microsoft.CSharp.RuntimeBinder.RuntimeBinderExtensions:IsEquivalentTo(System.Reflection.ParameterInfo,System.Reflection.ParameterInfo,System.Reflection.MethodBase,System.Reflection.MethodBase):bool
Top method improvements (percentages):
-16 (-11.35% of base) : 159193.dasm - System.Xml.Linq.XElement:AttributesEqual(System.Xml.Linq.XElement):bool:this
-16 (-9.64% of base) : 109061.dasm - Microsoft.CSharp.RuntimeBinder.RuntimeBinderExtensions:IsEquivalentTo(System.Reflection.ParameterInfo,System.Reflection.ParameterInfo,System.Reflection.MethodBase,System.Reflection.MethodBase):bool
-16 (-7.14% of base) : 113888.dasm - Microsoft.Extensions.Primitives.StringSegment:Equals(Microsoft.Extensions.Primitives.StringSegment,int):bool:this
3 total methods with Code Size differences (3 improved, 0 regressed), 0 unchanged.

danmoseley · 2021-03-12T23:16:43Z

Just curious, where is the hit in XElement.AttributesEqual? Is it somehow collapsing the distinct checks of name and value (both references)?

runtime/src/libraries/System.Private.Xml.Linq/src/System/Xml/Linq/XElement.cs

Lines 1918 to 1933 in ecc354a

privateboolAttributesEqual(XElemente)
{
XAttribute?a1=lastAttr;
XAttribute?a2=e.lastAttr;
if(a1!=null&&a2!=null)
{
do
{
a1=a1.next!;
a2=a2.next!;
if(a1.name!=a2.name||a1.value!=a2.value)returnfalse;
}while(a1!=lastAttr);
returna2==e.lastAttr;
}
returna1==null&&a2==null;
}

pentp · 2021-03-16T20:53:06Z

Transforming x!=0 || y!=0 to (x&y) !=0 is not correct. Did you mean (x|y) !=0?

I wonder if this optimization could actually cause regressions in already hand-optimized code by introducing additional register pressure and memory accesses? For example transforming some.x==0 && other.y==0 to (some.x | other.y)==0 requires an additional temporary register and unconditionally accesses another memory location that might not be in cache - this would be a regression if some.x==0 is usually true. It's not possible for the JIT to know if this is the case because this could depend on input data.

JulieLeeMSFT · 2021-03-16T22:59:37Z

Just curious, where is the hit in XElement.AttributesEqual? Is it somehow collapsing the distinct checks of name and value (both references)?

Working on other failing case. Will look into this later.
There is a Return statement pattern that cannot be folded like BB03 below.

***** BB01
STMT00000 (IL 0x000...0x001)
[000003] -----+------ * JTRUE void [000002] J----+-N---- \--* NE int [000000] -----+------ +--* LCL_VAR ref V00 arg0 [000001] -----+------ \--* CNS_INT ref null
------------ BB02 [003..008) (return), preds={BB01} succs={}
***** BB02
STMT00002 (IL 0x003...0x007)
[000012] -----+------ * RETURN int [000011] -----+------ \--* EQ int [000009] -----+------ +--* LCL_VAR ref V01 arg1 [000010] -----+------ \--* CNS_INT ref null
------------ BB03 [008..010) (return), preds={BB01} succs={}
***** BB03
STMT00001 (IL 0x008...0x00F)
[000008] --CXG+------ * RETURN int [000007] --CXG+------ \--* CAST int <- bool <- int
[000006] --CXG+------ \--* CALL r2r_ind int hackishModuleName.hackishMethodName
[000004] -----+------ this in rcx +--* LCL_VAR ref V00 arg0 [000005] -----+------ arg1 in rdx \--* LCL_VAR ref V01 arg1

JulieLeeMSFT · 2021-03-16T23:15:13Z

Transforming x!=0 || y!=0 to (x&y) !=0 is not correct. Did you mean (x|y) !=0?

Thanks for the good catch. I meant x!=0 && y!=0 to (x&y) !=0. Updated.

I wonder if this optimization could actually cause regressions in already hand-optimized code by introducing additional register pressure and memory accesses? For example transforming some.x==0 && other.y==0 to (some.x | other.y)==0 requires an additional temporary register and unconditionally accesses another memory location that might not be in cache - this would be a regression if some.x==0 is usually true. It's not possible for the JIT to know if this is the case because this could depend on input data.

We did this in order to reduce branching cost.
cc @AndyAyersMS what he thinks.

JulieLeeMSFT · 2021-03-17T03:12:32Z

Just curious, where is the hit in XElement.AttributesEqual? Is it somehow collapsing the distinct checks of name and value (both references)?

@danmoseley It is the last statement that is optimized.

return a1 == null && a2 == null;

Before optimization:

*** marking local variables in block BB08 (weight=0.50)
STMT00003 (IL 0x05D...0x05E)
[000015] -----+------ * JTRUE void [000014] J----+-N---- \--* NE int [000012] -----+------ +--* LCL_VAR ref V02 loc0 [000013] -----+------ \--* CNS_INT ref null
New refCnts for V02: refCnt = 8, refCntWtd = 22.50
*** marking local variables in block BB09 (weight=0.50)
STMT00005 (IL 0x060...0x064)
[000021] -----+------ * RETURN int [000020] -----+------ \--* EQ int [000018] -----+------ +--* LCL_VAR ref V03 loc1 [000019] -----+------ \--* CNS_INT ref null
New refCnts for V03: refCnt = 8, refCntWtd = 18.50
*** marking local variables in block BB10 (weight=0.50)
STMT00004 (IL 0x065...0x066)
[000017] -----+------ * RETURN int [000016] -----+------ \--* CNS_INT int 0

After optimization:

*************** In optOptimizeBools()
Folded boolean conditions of BB08, BB09 and BB10 to :
STMT00003 (IL 0x05D...0x05E)
[000015] -----+------ * RETURN int [000014] J----+-N---- \--* EQ int [000101] ------------ +--* OR long [000012] -----+------ | +--* LCL_VAR ref V02 loc0 ( 1, 1) [000018] ------------ | \--* LCL_VAR ref V03 loc1 [000013] -----+------ \--* CNS_INT long 0

JulieLeeMSFT · 2021-03-17T17:32:46Z

@AndyAyersMS all pri 0 tests passed. Please review this PR.
CC @dotnet/jit-contrib

pentp · 2021-03-17T20:12:51Z

Transforming x!=0 && y!=0 to (x&y) !=0 is also not correct (e.g., when x=1, y=2).

BruceForstall · 2021-03-17T20:49:03Z

Are there test cases in the CoreCLR pri-1 tests that hit all the cases you intend to optimize? If not, can you add a unit test (or multiple) that diffs will show this affecting?

JulieLeeMSFT · 2021-03-17T20:57:30Z

is also not correct (e.g., when x=1, y=2).

It is boolean optimization, so the values that are compared to is either 0 or 1.
Edit: This will not be optimized because the code will skip boolean optimization when it involves NOT operation.

JulieLeeMSFT · 2021-03-17T21:37:33Z

/azp list

azure-pipelines · 2021-03-17T21:37:38Z

CI/CD Pipelines for this repository: coreclr-gc-longrunning coreclr-gc-simulator runtime-coreclr outerloop runtime-coreclr jitstress runtime-coreclr jitstressregs runtime-coreclr jitstress2-jitstressregs runtime-coreclr r2r-extra runtime-coreclr jitstress-isas-x86 runtime-coreclr jitstress-isas-arm runtime-coreclr jitstressregs-x86 runtime-coreclr libraries-jitstressregs runtime-coreclr libraries-jitstress2-jitstressregs runtime-coreclr r2r runtime-coreclr runincontext runtime-coreclr crossgen2 runtime-libraries-coreclr outerloop runtime-libraries-coreclr outerloop-windows runtime-libraries-coreclr outerloop-linux runtime-libraries-coreclr outerloop-osx runtime runtime-libraries enterprise-linux runtime-libraries stress-http runtime-libraries stress-ssl runtime-dev-innerloop runtime-coreclr crossgen2 outerloop coreclr-release-outerloop-nightly sync-runtime-to-mono runtime-coreclr crossgen2-composite runtime-jit-experimental runtime-coreclr libraries-jitstress dotnet-linker-tests runtime-coreclr ilasm runtime-coreclr clrinterpreter runtime-coreclr crossgen2-composite gcstress runtime-libraries-mono outerloop runtime-staging

JulieLeeMSFT · 2021-03-17T21:41:38Z

/azp run runtime-coreclr outerloop

azure-pipelines · 2021-03-17T21:41:57Z

Azure Pipelines successfully started running 1 pipeline(s).

JulieLeeMSFT · 2021-03-17T22:38:27Z

/azp runtime-coreclr jitstress

JulieLeeMSFT · 2021-03-17T22:39:37Z

/azp run runtime-coreclr jitstress

azure-pipelines · 2021-03-17T22:39:51Z

Azure Pipelines successfully started running 1 pipeline(s).

AndyAyersMS

Left you some notes....

BruceForstall

left a few notes.

JulieLeeMSFT · 2021-03-22T04:08:46Z

/azp run runtime-coreclr outerloop

JulieLeeMSFT · 2021-03-22T04:09:01Z

/azp run runtime-coreclr jitstress

azure-pipelines · 2021-03-22T04:09:03Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2021-03-22T04:09:16Z

Azure Pipelines successfully started running 1 pipeline(s).

benaadams · 2021-03-22T05:14:48Z

CI is complaining about the C/C++ formatting

BruceForstall

It seems like a lot of code change for very few diffs. Why are there so few diffs?

Levi suggested handling cases like (a > 0) && (b > 0). Future work?

The optimization doesn't handle cases like bool b = a == 0 && b == 0 && c == 0 because the last block is a GT_ASG instead of GT_RETURN (for example). Future work?

BruceForstall

More comments.

Also, at some point, and after all changes are approved and final, make sure to run outerloop and jitstress pipelines.

JulieLeeMSFT · 2021-06-25T17:48:22Z

It seems like a lot of code change for very few diffs. Why are there so few diffs?

Is it because I had many commits?

Levi suggested handling cases like (a > 0) && (b > 0). Future work?

He suggested a < 0 || b < 0. It was not handled in this PR. I was planning to create another issue for that.

The optimization doesn't handle cases like bool b = a == 0 && b == 0 && c == 0 because the last block is a GT_ASG instead of GT_RETURN (for example). Future work?

This case was not handled. Will inlcude this case in the new issue together with a < 0 || b < 0.

JulieLeeMSFT · 2021-07-02T19:31:12Z

@BruceForstall @dotnet/jit-contrib I addressed BruceForstall's code review feedback. PTAL.

AndyAyersMS

Think this is getting close.

…and added test cases

JulieLeeMSFT · 2021-07-09T23:23:57Z

@AndyAyersMS @BruceForstall PTAL.

JulieLeeMSFT · 2021-07-11T20:37:24Z

/azp run runtime-coreclr outerloop

JulieLeeMSFT · 2021-07-11T20:37:40Z

/azp run runtime-coreclr jitstress

azure-pipelines · 2021-07-11T20:37:42Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2021-07-11T20:37:55Z

Azure Pipelines successfully started running 1 pipeline(s).

BruceForstall

I left a couple comments, but nothing blocking.

JulieLeeMSFT · 2021-07-13T21:13:36Z

I left a couple comments, but nothing blocking.

@BruceForstall @dotnet/jit-contrib PTAL. I incorporated Bruce's code review feedback.

JulieLeeMSFT · 2021-07-14T21:52:57Z

Test failure on OSX arm64 is not related to my change. It is infra issue. Merging the code.

JulieLeeMSFT added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 12, 2021

JulieLeeMSFT added this to the 6.0.0 milestone Mar 12, 2021

JulieLeeMSFT self-assigned this Mar 12, 2021

JulieLeeMSFT changed the title ~~"==0" optimization in Boolean logic #13573~~[Draft] "==0" optimization in Boolean logic #13573Mar 17, 2021

JulieLeeMSFT marked this pull request as draft March 17, 2021 22:23

This comment has been minimized.
Sign in to view

AndyAyersMS reviewed Mar 18, 2021
View reviewed changes

BruceForstall suggested changes Mar 19, 2021
View reviewed changes

Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated

runfoappBot mentioned this pull request Mar 22, 2021
OSX deprovision jaredpar/runfo#41
Closed

ewhapdxand others added 5 commits June 24, 2021 16:05

format patch
29dc719

Changed to substructure OptTestInfo within OptBoolsDisc
d02d18e

Cleaned up tree variables in OptBoolsDsc struct
ebf2010

Moved some methods for Boolean Optimization to OptBoolsDsc struct
a7cdf1c

Moved all private methods for Boolean Optimization to OptBoolsDsc struct
e3a6520

JulieLeeMSFT force-pushed the 13573 branch from 64601ce to e3a6520Compare June 24, 2021 23:12

BruceForstall suggested changes Jun 24, 2021
View reviewed changes

BruceForstall suggested changes Jun 25, 2021
View reviewed changes

runfoappBot mentioned this pull request Jun 25, 2021
Test failure System.Threading.Tests.PeriodicTimerTests.WaitForNextTickAsync_CanceledWaitThenWaitAgain_Succeeds #54713
Closed

Boolean Optimization: Handled code review feedback
5f02965

AndyAyersMS reviewed Jul 9, 2021
View reviewed changes

Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
Comment threadsrc/tests/JIT/opt/OptimizeBools/optboolsreturn.cs
Comment threadsrc/coreclr/jit/compiler.h Outdated

JulieLeeMSFT added 2 commits July 9, 2021 16:10

Optimize bools: hoisted jump destination check to optOptimizeBools() …
d14053e
…and added test cases

format patch
12c7a8a

BruceForstall approved these changes Jul 13, 2021
View reviewed changes

Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated
Comment threadsrc/coreclr/jit/optimizer.cpp Outdated

JulieLeeMSFT added 2 commits July 13, 2021 14:06

Moved initialization to OptBoolsDsc constructor
0e96608

format patch
1cdcdb8

JulieLeeMSFT mentioned this pull request Jul 13, 2021
JIT further Boolean logic optimization #55605
Closed

JulieLeeMSFT merged commit 8a34e76 into dotnet:mainJul 14, 2021

ManickaP mentioned this pull request Jul 20, 2021
[QUIC] Remove AppContext switch from S.N.Quic #56027
Merged

JulieLeeMSFT mentioned this pull request Aug 10, 2021
What's new in .NET 6 Preview 7 dotnet/core#6444
Closed

ghost locked as resolved and limited conversation to collaborators Aug 13, 2021

Uh oh!

Conversation

JulieLeeMSFT commented Mar 12, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danmoseley commented Mar 12, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pentp commented Mar 16, 2021

Uh oh!

JulieLeeMSFT commented Mar 16, 2021

Uh oh!

JulieLeeMSFT commented Mar 16, 2021

Uh oh!

JulieLeeMSFT commented Mar 17, 2021

Uh oh!

JulieLeeMSFT commented Mar 17, 2021

Uh oh!

pentp commented Mar 17, 2021

Uh oh!

BruceForstall commented Mar 17, 2021

Uh oh!

JulieLeeMSFT commented Mar 17, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JulieLeeMSFT commented Mar 17, 2021

Uh oh!

azure-pipelinesBot commented Mar 17, 2021

Uh oh!

JulieLeeMSFT commented Mar 17, 2021

Uh oh!

azure-pipelinesBot commented Mar 17, 2021

Uh oh!

JulieLeeMSFT commented Mar 17, 2021

Uh oh!

This comment has been minimized.

JulieLeeMSFT commented Mar 17, 2021

Uh oh!

azure-pipelinesBot commented Mar 17, 2021

Uh oh!

AndyAyersMS left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BruceForstall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JulieLeeMSFT commented Mar 22, 2021

Uh oh!

JulieLeeMSFT commented Mar 22, 2021

Uh oh!

azure-pipelinesBot commented Mar 22, 2021

Uh oh!

azure-pipelinesBot commented Mar 22, 2021

Uh oh!

benaadams commented Mar 22, 2021

Uh oh!

BruceForstall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JulieLeeMSFT commented Mar 12, 2021•
edited
Loading

danmoseley commented Mar 12, 2021•
edited
Loading

JulieLeeMSFT commented Mar 17, 2021•
edited
Loading