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

JIT: experimental changes to redundant branch opts #88527

Closed
wants to merge 4 commits into from

Conversation

AndyAyersMS
Copy link
Member

Recognize a particular case where a control flow pattern involving two BBJ_COND blocks with relops that test the same VNs can be simplified to a single relop in the dominating block.

As part of this, teach VN about some of the rudiments of boolean simplification (DeMorgan's laws) and how to simplify some NOT / AND / OR expressions involving relops.

Addresses some of the cases in #81220.

[This is an updated version of #83859]

Recognize a particular case where a control flow pattern involving two BBJ_COND
blocks with relops that test the same VNs can be simplified to a single relop
in the dominating block.

As part of this, teach VN about some of the rudiments of boolean simplification
(DeMorgan's laws)  and how to simplify some NOT / AND / OR expressions involving
relops.

Addresses some of the cases in dotnet#81220.
@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 Jul 7, 2023
@ghost ghost assigned AndyAyersMS Jul 7, 2023
@ghost
Copy link

ghost commented Jul 7, 2023

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

Issue Details

Recognize a particular case where a control flow pattern involving two BBJ_COND blocks with relops that test the same VNs can be simplified to a single relop in the dominating block.

As part of this, teach VN about some of the rudiments of boolean simplification (DeMorgan's laws) and how to simplify some NOT / AND / OR expressions involving relops.

Addresses some of the cases in #81220.

[This is an updated version of #83859]

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@hez2010
Copy link
Contributor

hez2010 commented Jul 8, 2023

Will this also fix #88025 (comment)?

@AndyAyersMS
Copy link
Member Author

Failure looks similar to #88579 (timeout in remote executor)

@AndyAyersMS
Copy link
Member Author

Will this also fix #88025 (comment)?

No, it doesn't fix that case (though it does a ton of flow graph simplification). Seems like for that we could try running a late tail merge...? That should be fairly cheap, let me see what happens.

@AndyAyersMS
Copy link
Member Author

Will this also fix #88025 (comment)?

No, it doesn't fix that case (though it does a ton of flow graph simplification). Seems like for that we could try running a late tail merge...? That should be fairly cheap, let me see what happens.

Well, not quite that simple, we are left with this sort of thing after running opts and fg cleanup:

***** BB22
STMT00082 ( INL22 @ 0x009[E-] ... ??? ) <- INL21 @ 0x010[E-] <- INL04 @ 0x030[E-] <- INL03 @ 0x000[E-] <- INLRT @ 0x020[--]
N002 (  5,  4) [000334] DA---------                         *  STORE_LCL_VAR int    V24 tmp24        d:3 $VN.Void
N001 (  1,  1) [000333] -----------                         \--*  CNS_INT   int    0 $41

***** BB22
STMT00073 ( INL21 @ 0x010[E-] ... ??? ) <- INL04 @ 0x030[E-] <- INL03 @ 0x000[E-] <- INLRT @ 0x020[--]
N002 (  8,  6) [000304] DA---------                         *  STORE_LCL_VAR int    V21 tmp21        d:4 $VN.Void
N001 (  4,  3) [000339] -----------                         \--*  LCL_VAR   bool   V24 tmp24        u:1 (last use) $301

***** BB22
STMT00023 ( INL04 @ 0x030[E-] ... ??? ) <- INL03 @ 0x000[E-] <- INLRT @ 0x020[--]
N002 (  8,  6) [000094] DA---------                         *  STORE_LCL_VAR int    V06 tmp6         d:3 $VN.Void
N001 (  4,  3) [000305] -----------                         \--*  LCL_VAR   bool   V21 tmp21        u:1 (last use) $302

------------ BB41 [020..021), preds={BB01} succs={BB42}

***** BB41
STMT00017 ( INL04 @ 0x047[E-] ... ??? ) <- INL03 @ 0x000[E-] <- INLRT @ 0x020[--]
N002 (  5,  4) [000065] DA---------                         *  STORE_LCL_VAR int    V06 tmp6         d:2 $VN.Void
N001 (  1,  1) [000064] -----------                         \--*  CNS_INT   int    0 $41

------------ BB42 [020..026) (return), preds={BB22,BB41} succs={}

***** BB42
STMT00106 ( ??? ... ??? )
N004 (  0,  0) [000496] DA---------                         *  STORE_LCL_VAR bool   V06 tmp6         d:1 $VN.Void
N003 (  0,  0) [000495] -----------                         \--*  PHI       bool   $303
N001 (  0,  0) [000504] ----------- pred BB40                  +--*  PHI_ARG   bool   V06 tmp6         u:3 $343
N002 (  0,  0) [000503] ----------- pred BB41                  \--*  PHI_ARG   bool   V06 tmp6         u:2 $41

***** BB42
STMT00004 ( 0x020[--] ... ??? )
N002 (  5,  4) [000016] -----------                         *  RETURN    int    $VN.Void
N001 (  4,  3) [000095] -----------                         \--*  LCL_VAR   bool   V06 tmp6         u:1 (last use) $303

As you can see this should all collapse to returning zero, but tail merging is hindered by the no-longer relevant PHI and the fact that the zero in BB22 needs to propagate through a few intermediate assignments.

You might wonder why we didn't realize that V06 was zero in BB22 sooner, it comes down to the fact that as RBO removes branches we're not able to take maximum advantage by updating VNs to reflect the more refined flow.

A more heavy-weight optimizer might rebuild SSA/VN after RBO, or run multiple passes of optimization, but that's not something we would do lightly...

@ghost
Copy link

ghost commented Aug 13, 2023

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 12, 2023
This pull request was closed.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants