Improve branch optimizer around implied relops #95234
Conversation
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsShould help with cases like: void Test(int x)
{
if (x > 100)
if (x > 10) // always true
Console.WriteLine();
}Ideally, it should be done in "redundant branch opt" phase, but it seems to be way more complicated so at least let's have it in assertprop for now.
|
EgorBo
force-pushed
the
implied-relop-assertprop
branch
from
ffa7d1a to
3f82e12
Compare
EgorBo
changed the title
|
/azp run runtime-coreclr outerloop, runtime-coreclr jitstress, Fuzzlyn |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
PTAL @AndyAyersMS @dotnet/jit-contrib Diffs (not too big but I have some follow-up ideas how to extend them) |
| // BB4: | ||
| // return; | ||
|
|
||
| // Check whether the dominating compare being "false" implies the dominated compare is known |
There was a problem hiding this comment.
The reversal/negation here is a bit tricky, but I think I follow it.
Seems like there's a complementary case where you don't negate the upper relop and then may instead have canInferFromTrue abilities?
There was a problem hiding this comment.
Yeah, basically there are two cases:
canInferFromFalse:
if (x > 100)
{
if (x > 10)
{
...
}
}canInferFromTrue:
if (x > 100)
{
...
}
if (x > 10)
{
...
}I'll check separately the 2nd case (last I tried it added very little diffs)
How many of those diffs represent places code itself should be cleaned up? Like your example. |
I inspected a few diffs and they were inlining-related so I wasn't able to find an obvious case that can be cleaned up in C# |
|
Hm.. arm failures look related and they're floating point related... soft floats? |
…1 into implied-relop-assertprop
|
Since the last review I found a small bug - I replaced |
4 participants
Should help with cases like:
; Method Program:Test(int):this (FullOpts) sub rsp, 40 cmp edx, 100 jle SHORT G_M52364_IG04 - cmp edx, 10 - jle SHORT G_M52364_IG04 call [System.Console:WriteLine()] G_M52364_IG04: nop add rsp, 40 ret -; Total bytes of code: 26 +; Total bytes of code: 21Ideally, it should be done in "redundant branch opt" phase, but it seems to be way more complicated so at least let's have it in assertprop for now: EgorBo@cdbaac0