Add additional binary operations into the RangeCheck analysis. #61662

anthonycanino · 2021-11-16T09:55:32Z

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.

ghost · 2021-11-16T09:55:39Z

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 · 2021-11-17T11:56:37Z

/azp list

azure-pipelines · 2021-11-17T11:56:42Z

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

anthonycanino · 2021-11-17T11:58:16Z

/azp run runtime

azure-pipelines · 2021-11-17T11:58:22Z

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

src/coreclr/jit/rangecheck.h

Tests catch some edge cases with multiplcation overflow IF the overflow detection isn't implemented correctly.

anthonycanino · 2021-11-18T21:42:05Z

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

Understood. Fixed. Thank you.

jakobbotsch · 2021-11-18T21:56:40Z

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

anthonycanino · 2021-11-30T20:45:38Z

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

jakobbotsch · 2021-11-30T21:37:18Z

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

Resolved, but have not checked remaining changes

JulieLeeMSFT · 2021-12-01T04:16:52Z

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

@jakobbotsch I am checking with the team.

src/coreclr/jit/rangecheck.cpp

EgorBo · 2021-12-06T10:26:23Z

/azp run Fuzzlyn

azure-pipelines · 2021-12-06T10:26:39Z

Azure Pipelines successfully started running 1 pipeline(s).

kunalspathak · 2021-12-06T16:10:54Z

/azp run Antigen

azure-pipelines · 2021-12-06T16:11:09Z

Azure Pipelines successfully started running 1 pipeline(s).

Co-authored-by: Egor Bogatov <egorbo@gmail.com>

anthonycanino · 2021-12-06T19:17:19Z

@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 · 2021-12-06T19:42:44Z

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

anthonycanino · 2021-12-07T18:52:56Z

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

kunalspathak · 2021-12-07T18:54:02Z

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

EgorBo · 2021-12-09T11:12:09Z

@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 · 2021-12-09T12:31:27Z

@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 · 2021-12-09T14:55:35Z

@anthonycanino thank you for the contribution!

anthonycanino · 2021-12-09T17:49:24Z

Thanks for the review!

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

anthonycanino mentioned this pull request Nov 16, 2021

Reduce repeated variable add operations to a single multiply. #61663

Merged

anthonycanino force-pushed the anthony/rcheck branch from 9ba6051 to ebc2288 Compare November 17, 2021 11:59

anthonycanino marked this pull request as ready for review November 17, 2021 14:45

jakobbotsch previously requested changes Nov 17, 2021

View reviewed changes

src/coreclr/jit/rangecheck.h Outdated Show resolved Hide resolved

anthonycanino added 2 commits November 18, 2021 12:22

RangeCheck multiply overflow fix and tests.

162ed95

Tests catch some edge cases with multiplcation overflow IF the overflow detection isn't implemented correctly.

Use CheckedOps::Signed instead of false.

33a3b43

runfoapp bot mentioned this pull request Nov 19, 2021

system.text.regularexpressions.tests.regexmatchtests.match_cachedpattern_newtimeoutapplies #61794

Closed

Reduced overflow test array size to more reasonable value.

7872399

EgorBo self-requested a review December 1, 2021 14:42

JulieLeeMSFT assigned EgorBo Dec 1, 2021

JulieLeeMSFT added this to the 7.0.0 milestone Dec 1, 2021

EgorBo reviewed Dec 6, 2021

View reviewed changes

src/coreclr/jit/rangecheck.cpp Outdated Show resolved Hide resolved

EgorBo reviewed Dec 6, 2021

View reviewed changes

src/coreclr/jit/rangecheck.cpp Outdated Show resolved Hide resolved

anthonycanino and others added 2 commits December 6, 2021 14:10

Update src/coreclr/jit/rangecheck.cpp

9b4470c

Co-authored-by: Egor Bogatov <egorbo@gmail.com>

Update src/coreclr/jit/rangecheck.cpp

01e4faa

Co-authored-by: Egor Bogatov <egorbo@gmail.com>

EgorBo approved these changes Dec 9, 2021

View reviewed changes

EgorBo merged commit fd28c76 into dotnet:main Dec 9, 2021

ghost locked as resolved and limited conversation to collaborators Jan 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add additional binary operations into the RangeCheck analysis. #61662

Add additional binary operations into the RangeCheck analysis. #61662

anthonycanino commented Nov 16, 2021

ghost commented Nov 16, 2021

Changes

Discussion

Results

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

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

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

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

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

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

anthonycanino commented Nov 17, 2021

azure-pipelines bot commented Nov 17, 2021

anthonycanino commented Nov 17, 2021

azure-pipelines bot commented Nov 17, 2021

anthonycanino commented Nov 18, 2021

jakobbotsch commented Nov 18, 2021

anthonycanino commented Nov 30, 2021

jakobbotsch commented Nov 30, 2021

JulieLeeMSFT commented Dec 1, 2021

EgorBo commented Dec 6, 2021

azure-pipelines bot commented Dec 6, 2021

kunalspathak commented Dec 6, 2021

azure-pipelines bot commented Dec 6, 2021

anthonycanino commented Dec 6, 2021

kunalspathak commented Dec 6, 2021

anthonycanino commented Dec 7, 2021

kunalspathak commented Dec 7, 2021

EgorBo commented Dec 9, 2021

anthonycanino commented Dec 9, 2021

EgorBo commented Dec 9, 2021

anthonycanino commented Dec 9, 2021

Add additional binary operations into the RangeCheck analysis. #61662

Add additional binary operations into the RangeCheck analysis. #61662

Conversation

anthonycanino commented Nov 16, 2021

Changes

Discussion

Results

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

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

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

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

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

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

ghost commented Nov 16, 2021

Changes

Discussion

Results

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

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

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

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

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

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

anthonycanino commented Nov 17, 2021

azure-pipelines bot commented Nov 17, 2021

anthonycanino commented Nov 17, 2021

azure-pipelines bot commented Nov 17, 2021

anthonycanino commented Nov 18, 2021

jakobbotsch commented Nov 18, 2021

anthonycanino commented Nov 30, 2021

jakobbotsch commented Nov 30, 2021

JulieLeeMSFT commented Dec 1, 2021

EgorBo commented Dec 6, 2021

azure-pipelines bot commented Dec 6, 2021

kunalspathak commented Dec 6, 2021

azure-pipelines bot commented Dec 6, 2021

anthonycanino commented Dec 6, 2021

kunalspathak commented Dec 6, 2021

anthonycanino commented Dec 7, 2021

kunalspathak commented Dec 7, 2021

EgorBo commented Dec 9, 2021

anthonycanino commented Dec 9, 2021

EgorBo commented Dec 9, 2021

anthonycanino commented Dec 9, 2021