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

Add additional binary operations into the RangeCheck analysis. #61662

Merged
merged 6 commits into from
Dec 9, 2021

Conversation

anthonycanino
Copy link
Contributor

Changes

GT_LSH and GT_MUL are now covered in the range analysis check. This
allows to catch and eliminate the range check for cases like

ReadOnlySpan<byte> readOnlySpan => new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

byte byt = 0;
for (int i = 0; i < 5; i++)
{
  byt = readOnlySpan[i * 3];
  // ...
}

or

bool[] flags = new bool[Size + 1];

for (int i = 2; i <= Size; i++)
{
  for (int k = i * 2; k <= Size; k += i)
  {
    flags[k] = false;
  }
}

Note that without this change, the previous snippet would not eliminate
the range check on flags[k], but the equivalent snippet would

for (int i = 2; i <= Size; i++)
{
  for (int k = i + i; k <= Size; k += i)
  {
    flags[k] = false;
  }
}

as additional was implemented in the range check analysis, but multiply
was not.

Discussion

I stumbled on this limitation when implementing #34938. Essentially, the strength reduction that I wrote was optimizing expressions like arr[i + i] to arr[i * 2], which would then become arr[i << 1]. The issue is that in some cases, this was preventing the RangeCheck analysis from eliminating a range check, so it was a slight regression.

I have implemented GT_MUL and GT_LSH into the range check analysis and I am looking for some feedback, especially on whether other operations should be included in this change as well.

Results

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


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 13670517 (overridden on cmd)
Total bytes of diff: 13670481 (overridden on cmd)
Total bytes of delta: -36 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -12 : 4829.dasm (-3.49% of base)
         -12 : 13972.dasm (-3.49% of base)
         -12 : 6069.dasm (-3.49% of base)

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

Top method improvements (bytes):
         -12 (-3.49% of base) : 4829.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this
         -12 (-3.49% of base) : 13972.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this
         -12 (-3.49% of base) : 6069.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this

Top method improvements (percentages):
         -12 (-3.49% of base) : 4829.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this
         -12 (-3.49% of base) : 13972.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this
         -12 (-3.49% of base) : 6069.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this

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


benchmarks.run.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 7151027 (overridden on cmd)
Total bytes of diff: 7150965 (overridden on cmd)
Total bytes of delta: -62 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -31 : 23151.dasm (-11.19% of base)
         -19 : 4875.dasm (-7.17% of base)
         -12 : 4876.dasm (-3.49% of base)

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

Top method improvements (bytes):
         -31 (-11.19% of base) : 23151.dasm - System.Globalization.HebrewCalendar:GetLunarMonthDay(int,DateBuffer):int
         -19 (-7.17% of base) : 4875.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -12 (-3.49% of base) : 4876.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this

Top method improvements (percentages):
         -31 (-11.19% of base) : 23151.dasm - System.Globalization.HebrewCalendar:GetLunarMonthDay(int,DateBuffer):int
         -19 (-7.17% of base) : 4875.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -12 (-3.49% of base) : 4876.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this

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


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


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 124162883 (overridden on cmd)
Total bytes of diff: 124162693 (overridden on cmd)
Total bytes of delta: -190 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -19 : 197466.dasm (-46.34% of base)
         -19 : 197418.dasm (-46.34% of base)
         -19 : 197474.dasm (-46.34% of base)
         -19 : 197438.dasm (-46.34% of base)
         -19 : 197454.dasm (-46.34% of base)
         -19 : 197430.dasm (-46.34% of base)
         -19 : 197424.dasm (-46.34% of base)
         -19 : 197460.dasm (-46.34% of base)
         -19 : 197432.dasm (-45.24% of base)
         -19 : 197468.dasm (-45.24% of base)

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

Top method improvements (bytes):
         -19 (-46.34% of base) : 197454.dasm - Tests:Test337_cns(int):ubyte
         -19 (-46.34% of base) : 197460.dasm - Tests:Test340_cns(int):ubyte
         -19 (-46.34% of base) : 197466.dasm - Tests:Test343_cns(int):ubyte
         -19 (-45.24% of base) : 197468.dasm - Tests:Test344_cns(int):ubyte
         -19 (-46.34% of base) : 197474.dasm - Tests:Test347_cns(int):ubyte
         -19 (-46.34% of base) : 197418.dasm - Tests:Test351_cns(int):ubyte
         -19 (-46.34% of base) : 197424.dasm - Tests:Test354_cns(int):ubyte
         -19 (-46.34% of base) : 197430.dasm - Tests:Test357_cns(int):ubyte
         -19 (-45.24% of base) : 197432.dasm - Tests:Test358_cns(int):ubyte
         -19 (-46.34% of base) : 197438.dasm - Tests:Test361_cns(int):ubyte

Top method improvements (percentages):
         -19 (-46.34% of base) : 197454.dasm - Tests:Test337_cns(int):ubyte
         -19 (-46.34% of base) : 197460.dasm - Tests:Test340_cns(int):ubyte
         -19 (-46.34% of base) : 197466.dasm - Tests:Test343_cns(int):ubyte
         -19 (-46.34% of base) : 197474.dasm - Tests:Test347_cns(int):ubyte
         -19 (-46.34% of base) : 197418.dasm - Tests:Test351_cns(int):ubyte
         -19 (-46.34% of base) : 197424.dasm - Tests:Test354_cns(int):ubyte
         -19 (-46.34% of base) : 197430.dasm - Tests:Test357_cns(int):ubyte
         -19 (-46.34% of base) : 197438.dasm - Tests:Test361_cns(int):ubyte
         -19 (-45.24% of base) : 197468.dasm - Tests:Test344_cns(int):ubyte
         -19 (-45.24% of base) : 197432.dasm - Tests:Test358_cns(int):ubyte

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


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


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 34283537 (overridden on cmd)
Total bytes of diff: 34283498 (overridden on cmd)
Total bytes of delta: -39 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -31 : 116985.dasm (-11.83% of base)
          -8 : 153703.dasm (-0.80% of base)

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

Top method improvements (bytes):
         -31 (-11.83% of base) : 116985.dasm - System.Globalization.HebrewCalendar:GetLunarMonthDay(int,System.Globalization.HebrewCalendar+DateBuffer):int
          -8 (-0.80% of base) : 153703.dasm - System.Runtime.Serialization.DataContract:ComputeHash(System.Byte[]):System.Byte[]

Top method improvements (percentages):
         -31 (-11.83% of base) : 116985.dasm - System.Globalization.HebrewCalendar:GetLunarMonthDay(int,System.Globalization.HebrewCalendar+DateBuffer):int
          -8 (-0.80% of base) : 153703.dasm - System.Runtime.Serialization.DataContract:ComputeHash(System.Byte[]):System.Byte[]

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


libraries.pmi.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 45344459 (overridden on cmd)
Total bytes of diff: 45344420 (overridden on cmd)
Total bytes of delta: -39 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -19 : 207710.dasm (-7.17% of base)
         -12 : 207706.dasm (-3.49% of base)
          -8 : 126617.dasm (-0.78% of base)

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

Top method improvements (bytes):
         -19 (-7.17% of base) : 207710.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -12 (-3.49% of base) : 207706.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
          -8 (-0.78% of base) : 126617.dasm - System.Runtime.Serialization.DataContract:ComputeHash(System.Byte[]):System.Byte[]

Top method improvements (percentages):
         -19 (-7.17% of base) : 207710.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -12 (-3.49% of base) : 207706.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
          -8 (-0.78% of base) : 126617.dasm - System.Runtime.Serialization.DataContract:ComputeHash(System.Byte[]):System.Byte[]

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


libraries_tests.pmi.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 114254056 (overridden on cmd)
Total bytes of diff: 114253969 (overridden on cmd)
Total bytes of delta: -87 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -19 : 304736.dasm (-7.17% of base)
         -19 : 304440.dasm (-7.17% of base)
         -13 : 279719.dasm (-5.80% of base)
         -12 : 304436.dasm (-3.49% of base)
         -12 : 303299.dasm (-3.23% of base)
         -12 : 304732.dasm (-3.49% of base)

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

Top method improvements (bytes):
         -19 (-7.17% of base) : 304736.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -19 (-7.17% of base) : 304440.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -13 (-5.80% of base) : 279719.dasm - System.ConsoleManualTests:Colors()
         -12 (-3.49% of base) : 304436.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
         -12 (-3.49% of base) : 304732.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
         -12 (-3.23% of base) : 303299.dasm - System.Net.NetworkInformation.StringParsingHelpers:ParseIPv6HexString(System.ReadOnlySpan`1[Char],bool):System.Net.IPAddress

Top method improvements (percentages):
         -19 (-7.17% of base) : 304736.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -19 (-7.17% of base) : 304440.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -13 (-5.80% of base) : 279719.dasm - System.ConsoleManualTests:Colors()
         -12 (-3.49% of base) : 304436.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
         -12 (-3.49% of base) : 304732.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
         -12 (-3.23% of base) : 303299.dasm - System.Net.NetworkInformation.StringParsingHelpers:ParseIPv6HexString(System.ReadOnlySpan`1[Char],bool):System.Net.IPAddress

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


@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 Nov 16, 2021
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Nov 16, 2021
@ghost
Copy link

ghost commented Nov 16, 2021

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

Issue Details

Changes

GT_LSH and GT_MUL are now covered in the range analysis check. This
allows to catch and eliminate the range check for cases like

ReadOnlySpan<byte> readOnlySpan => new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

byte byt = 0;
for (int i = 0; i < 5; i++)
{
  byt = readOnlySpan[i * 3];
  // ...
}

or

bool[] flags = new bool[Size + 1];

for (int i = 2; i <= Size; i++)
{
  for (int k = i * 2; k <= Size; k += i)
  {
    flags[k] = false;
  }
}

Note that without this change, the previous snippet would not eliminate
the range check on flags[k], but the equivalent snippet would

for (int i = 2; i <= Size; i++)
{
  for (int k = i + i; k <= Size; k += i)
  {
    flags[k] = false;
  }
}

as additional was implemented in the range check analysis, but multiply
was not.

Discussion

I stumbled on this limitation when implementing #34938. Essentially, the strength reduction that I wrote was optimizing expressions like arr[i + i] to arr[i * 2], which would then become arr[i << 1]. The issue is that in some cases, this was preventing the RangeCheck analysis from eliminating a range check, so it was a slight regression.

I have implemented GT_MUL and GT_LSH into the range check analysis and I am looking for some feedback, especially on whether other operations should be included in this change as well.

Results

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


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 13670517 (overridden on cmd)
Total bytes of diff: 13670481 (overridden on cmd)
Total bytes of delta: -36 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -12 : 4829.dasm (-3.49% of base)
         -12 : 13972.dasm (-3.49% of base)
         -12 : 6069.dasm (-3.49% of base)

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

Top method improvements (bytes):
         -12 (-3.49% of base) : 4829.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this
         -12 (-3.49% of base) : 13972.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this
         -12 (-3.49% of base) : 6069.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this

Top method improvements (percentages):
         -12 (-3.49% of base) : 4829.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this
         -12 (-3.49% of base) : 13972.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this
         -12 (-3.49% of base) : 6069.dasm - IPAddress:.ctor(ReadOnlySpan`1,long):this

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


benchmarks.run.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 7151027 (overridden on cmd)
Total bytes of diff: 7150965 (overridden on cmd)
Total bytes of delta: -62 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -31 : 23151.dasm (-11.19% of base)
         -19 : 4875.dasm (-7.17% of base)
         -12 : 4876.dasm (-3.49% of base)

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

Top method improvements (bytes):
         -31 (-11.19% of base) : 23151.dasm - System.Globalization.HebrewCalendar:GetLunarMonthDay(int,DateBuffer):int
         -19 (-7.17% of base) : 4875.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -12 (-3.49% of base) : 4876.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this

Top method improvements (percentages):
         -31 (-11.19% of base) : 23151.dasm - System.Globalization.HebrewCalendar:GetLunarMonthDay(int,DateBuffer):int
         -19 (-7.17% of base) : 4875.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -12 (-3.49% of base) : 4876.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this

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


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


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 124162883 (overridden on cmd)
Total bytes of diff: 124162693 (overridden on cmd)
Total bytes of delta: -190 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -19 : 197466.dasm (-46.34% of base)
         -19 : 197418.dasm (-46.34% of base)
         -19 : 197474.dasm (-46.34% of base)
         -19 : 197438.dasm (-46.34% of base)
         -19 : 197454.dasm (-46.34% of base)
         -19 : 197430.dasm (-46.34% of base)
         -19 : 197424.dasm (-46.34% of base)
         -19 : 197460.dasm (-46.34% of base)
         -19 : 197432.dasm (-45.24% of base)
         -19 : 197468.dasm (-45.24% of base)

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

Top method improvements (bytes):
         -19 (-46.34% of base) : 197454.dasm - Tests:Test337_cns(int):ubyte
         -19 (-46.34% of base) : 197460.dasm - Tests:Test340_cns(int):ubyte
         -19 (-46.34% of base) : 197466.dasm - Tests:Test343_cns(int):ubyte
         -19 (-45.24% of base) : 197468.dasm - Tests:Test344_cns(int):ubyte
         -19 (-46.34% of base) : 197474.dasm - Tests:Test347_cns(int):ubyte
         -19 (-46.34% of base) : 197418.dasm - Tests:Test351_cns(int):ubyte
         -19 (-46.34% of base) : 197424.dasm - Tests:Test354_cns(int):ubyte
         -19 (-46.34% of base) : 197430.dasm - Tests:Test357_cns(int):ubyte
         -19 (-45.24% of base) : 197432.dasm - Tests:Test358_cns(int):ubyte
         -19 (-46.34% of base) : 197438.dasm - Tests:Test361_cns(int):ubyte

Top method improvements (percentages):
         -19 (-46.34% of base) : 197454.dasm - Tests:Test337_cns(int):ubyte
         -19 (-46.34% of base) : 197460.dasm - Tests:Test340_cns(int):ubyte
         -19 (-46.34% of base) : 197466.dasm - Tests:Test343_cns(int):ubyte
         -19 (-46.34% of base) : 197474.dasm - Tests:Test347_cns(int):ubyte
         -19 (-46.34% of base) : 197418.dasm - Tests:Test351_cns(int):ubyte
         -19 (-46.34% of base) : 197424.dasm - Tests:Test354_cns(int):ubyte
         -19 (-46.34% of base) : 197430.dasm - Tests:Test357_cns(int):ubyte
         -19 (-46.34% of base) : 197438.dasm - Tests:Test361_cns(int):ubyte
         -19 (-45.24% of base) : 197468.dasm - Tests:Test344_cns(int):ubyte
         -19 (-45.24% of base) : 197432.dasm - Tests:Test358_cns(int):ubyte

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


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


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 34283537 (overridden on cmd)
Total bytes of diff: 34283498 (overridden on cmd)
Total bytes of delta: -39 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -31 : 116985.dasm (-11.83% of base)
          -8 : 153703.dasm (-0.80% of base)

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

Top method improvements (bytes):
         -31 (-11.83% of base) : 116985.dasm - System.Globalization.HebrewCalendar:GetLunarMonthDay(int,System.Globalization.HebrewCalendar+DateBuffer):int
          -8 (-0.80% of base) : 153703.dasm - System.Runtime.Serialization.DataContract:ComputeHash(System.Byte[]):System.Byte[]

Top method improvements (percentages):
         -31 (-11.83% of base) : 116985.dasm - System.Globalization.HebrewCalendar:GetLunarMonthDay(int,System.Globalization.HebrewCalendar+DateBuffer):int
          -8 (-0.80% of base) : 153703.dasm - System.Runtime.Serialization.DataContract:ComputeHash(System.Byte[]):System.Byte[]

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


libraries.pmi.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 45344459 (overridden on cmd)
Total bytes of diff: 45344420 (overridden on cmd)
Total bytes of delta: -39 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -19 : 207710.dasm (-7.17% of base)
         -12 : 207706.dasm (-3.49% of base)
          -8 : 126617.dasm (-0.78% of base)

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

Top method improvements (bytes):
         -19 (-7.17% of base) : 207710.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -12 (-3.49% of base) : 207706.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
          -8 (-0.78% of base) : 126617.dasm - System.Runtime.Serialization.DataContract:ComputeHash(System.Byte[]):System.Byte[]

Top method improvements (percentages):
         -19 (-7.17% of base) : 207710.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -12 (-3.49% of base) : 207706.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
          -8 (-0.78% of base) : 126617.dasm - System.Runtime.Serialization.DataContract:ComputeHash(System.Byte[]):System.Byte[]

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


libraries_tests.pmi.windows.x64.checked.mch:


Summary of Code Size diffs:
(Lower is better)

Total bytes of base: 114254056 (overridden on cmd)
Total bytes of diff: 114253969 (overridden on cmd)
Total bytes of delta: -87 (-0.00 % of base)
    diff is an improvement.
    relative diff is an improvement.
Detail diffs


Top file improvements (bytes):
         -19 : 304736.dasm (-7.17% of base)
         -19 : 304440.dasm (-7.17% of base)
         -13 : 279719.dasm (-5.80% of base)
         -12 : 304436.dasm (-3.49% of base)
         -12 : 303299.dasm (-3.23% of base)
         -12 : 304732.dasm (-3.49% of base)

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

Top method improvements (bytes):
         -19 (-7.17% of base) : 304736.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -19 (-7.17% of base) : 304440.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -13 (-5.80% of base) : 279719.dasm - System.ConsoleManualTests:Colors()
         -12 (-3.49% of base) : 304436.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
         -12 (-3.49% of base) : 304732.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
         -12 (-3.23% of base) : 303299.dasm - System.Net.NetworkInformation.StringParsingHelpers:ParseIPv6HexString(System.ReadOnlySpan`1[Char],bool):System.Net.IPAddress

Top method improvements (percentages):
         -19 (-7.17% of base) : 304736.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -19 (-7.17% of base) : 304440.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte]):this
         -13 (-5.80% of base) : 279719.dasm - System.ConsoleManualTests:Colors()
         -12 (-3.49% of base) : 304436.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
         -12 (-3.49% of base) : 304732.dasm - System.Net.IPAddress:.ctor(System.ReadOnlySpan`1[Byte],long):this
         -12 (-3.23% of base) : 303299.dasm - System.Net.NetworkInformation.StringParsingHelpers:ParseIPv6HexString(System.ReadOnlySpan`1[Char],bool):System.Net.IPAddress

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


Author: anthonycanino
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

GT_LSH and GT_MUL are now covered in the range analysis check. This
allows to catch and eliminate the range check for cases like

```
ReadOnlySpan<byte> readOnlySpan => new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

byte byt = 0;
for (int i = 0; i < 5; i++)
{
  byt = readOnlySpan[i * 3];
  // ...
}
```

or

```
bool[] flags = new bool[Size + 1];

for (int i = 2; i <= Size; i++)
{
  for (int k = i * 2; k <= Size; k += i)
  {
    flags[k] = false;
  }
}
```

Note that without this change, the previous snippet would not eliminate
the range check on `flags[k]`, but the equivalent snippet would

```
for (int i = 2; i <= Size; i++)
{
  for (int k = i + i; k <= Size; k += i)
  {
    flags[k] = false;
  }
}

```

as additional was implemented in the range check analysis, but multiply
was not.
@anthonycanino
Copy link
Contributor Author

/azp list

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 61662 in repo dotnet/runtime

@anthonycanino
Copy link
Contributor Author

/azp run runtime

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 61662 in repo dotnet/runtime

@anthonycanino anthonycanino marked this pull request as ready for review November 17, 2021 14:45
src/coreclr/jit/rangecheck.h Outdated Show resolved Hide resolved
Tests catch some edge cases with multiplcation overflow IF
the overflow detection isn't implemented correctly.
@anthonycanino
Copy link
Contributor Author

The pattern with CheckedOps is to use CheckedOps::Signed/Unsigned for readability (instead of the "bare" false/true).

Understood. Fixed. Thank you.

@jakobbotsch
Copy link
Member

cc @dotnet/jit-contrib - is there someone familiar with range check elimination that want to give this a lookover?

@anthonycanino
Copy link
Contributor Author

@jakobbotsch can I get an update on this? Happy to continue to make changes per review.

@jakobbotsch
Copy link
Member

@anthonycanino Sorry, this fell off my radar. @JulieLeeMSFT, do you mind assigning someone here?

@jakobbotsch jakobbotsch dismissed their stale review November 30, 2021 22:32

Resolved, but have not checked remaining changes

@JulieLeeMSFT
Copy link
Member

@anthonycanino Sorry, this fell off my radar. @JulieLeeMSFT, do you mind assigning someone here?

@jakobbotsch I am checking with the team.

@EgorBo EgorBo self-requested a review December 1, 2021 14:42
@JulieLeeMSFT JulieLeeMSFT added this to the 7.0.0 milestone Dec 1, 2021
@EgorBo
Copy link
Member

EgorBo commented Dec 6, 2021

/azp run Fuzzlyn

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kunalspathak
Copy link
Member

/azp run Antigen

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

anthonycanino and others added 2 commits December 6, 2021 14:10
Co-authored-by: Egor Bogatov <egorbo@gmail.com>
Co-authored-by: Egor Bogatov <egorbo@gmail.com>
@anthonycanino
Copy link
Contributor Author

@EgorBo @kunalspathak thanks for the extra tests. Not sure if when I comited the suggested changes it erases the results from the check list but here are the Antigen and Fuzzlyn pipeline results for quick reference.

https://dev.azure.com/dnceng/public/_build/results?buildId=1498464&view=results
https://dev.azure.com/dnceng/public/_build/results?buildId=1498926&view=results

How do I in general interpret these? Do I presume the assertion failures are related to my changes and work backwards from there? I've not not to interpret the fuzzer results yet on my other PRs. Thanks!

@kunalspathak
Copy link
Member

I went through the results, and they seem to be existing issues, nothing specific came up for this PR.

@anthonycanino
Copy link
Contributor Author

Hi folks, is there any further feedback or thoughts on this PR? Do we need more changes?

@kunalspathak
Copy link
Member

@EgorBo - Can you check if this is ready to merge?

@EgorBo
Copy link
Member

EgorBo commented Dec 9, 2021

@anthonycanino LGTM, thanks, the only note that the initial description mentions

ReadOnlySpan<byte> readOnlySpan => new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

byte byt = 0;
for (int i = 0; i < 5; i++)
{
  byt = readOnlySpan[i * 3];
  // ...
}

but I see that it's not handled currently, am I right?

@anthonycanino
Copy link
Contributor Author

@anthonycanino LGTM, thanks, the only note that the initial description mentions

ReadOnlySpan<byte> readOnlySpan => new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };

byte byt = 0;
for (int i = 0; i < 5; i++)
{
  byt = readOnlySpan[i * 3];
  // ...
}

but I see that it's not handled currently, am I right?

It should be handled, per the code at ebc2288#diff-44e9b099c79041ac875722c02c7de2c836313b3a22d53c91214c2a74fdeaa1cfR1003-R1018

@EgorBo EgorBo merged commit fd28c76 into dotnet:main Dec 9, 2021
@EgorBo
Copy link
Member

EgorBo commented Dec 9, 2021

@anthonycanino thank you for the contribution!

@anthonycanino
Copy link
Contributor Author

Thanks for the review!

@ghost ghost locked as resolved and limited conversation to collaborators Jan 8, 2022
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 community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants