Optimize if range check to replace jumps to bit operation

[JulieLeeMSFT]

It detects if statement with specifc range check patterns and optimizes them to use bit operation instead of jumps for each pattern.

Pattern 1 optimized in this PR:

staticboolIsReservedPattern(inta){if(a==97||a==98||a==100||a==102)returntrue;returnfalse;}

Before:

cmpecx,97je SHORT G_M14484_IG04cmpecx,98je SHORT G_M14484_IG04cmpecx,100je SHORT G_M14484_IG04cmpecx,102jne SHORT G_M14484_IG06G_M14484_IG04: moveax,1retG_M14484_IG06: xoreax,eaxret

After:

subecx,97cmpecx,5ja SHORT G_M14484_IG05moveax,43bteax,ecxjae SHORT G_M14484_IG05moveax,1retG_M14484_IG05: xoreax,eaxret

Contributes to #8418.

[ghost]

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

Issue Details

---

It detects if statement with specifc range check patterns and optimizes them to use bit operation instead of jumps for each pattern.

Pattern 1 optimized in this PR:

 static bool IsReservedPattern(int a)
{
if (a == 97 || a == 98 || a == 100 || a == 102)
return true;
return false;
}

Before:

 cmp ecx, 97
je SHORT G_M14484_IG04
cmp ecx, 98
je SHORT G_M14484_IG04
cmp ecx, 100
je SHORT G_M14484_IG04
cmp ecx, 102
jne SHORT G_M14484_IG06
G_M14484_IG04: mov eax, 1
ret G_M14484_IG06: xor eax, eax
ret 

After:

 sub ecx, 97
cmp ecx, 5
ja SHORT G_M14484_IG05
mov eax, 43
bt eax, ecx
jae SHORT G_M14484_IG05
mov eax, 1
ret G_M14484_IG05: xor eax, eax
ret 

Contributes to #8418.

Author:	JulieLeeMSFT
Assignees:	JulieLeeMSFT
Labels:	area-CodeGen-coreclr
Milestone:	-

[JulieLeeMSFT]

Reference: https://egorbo.com/llvm-range-checks.html

[danmoseley]

@JulieLeeMSFT you can set the code blocks to colorize, I modified your top post, basically just put C# or asm immediately after the first three `

[danmoseley]

It's an interesting trick we can use in c# and the range doesn't have to be contiguous (!), simply no more than register width between highest and lowest values. @stephentoub does Regex take advantage of this already? Perhaps it's an element of how searching is already optimized?

[danmoseley]

@MihaZupan but assuming this is already well known.

[stephentoub]

@stephentoub does Regex take advantage of this already? Perhaps it's an element of how searching is already optimized?

In concept. It generates a branch-free version, e.g. for the set [abdf] that's being checked for in IsReservedPattern above, it emits the equivalent of this:

staticboolIsReservedPattern(inta){uintcharMinusLowUInt32;return((int)((0xD4000000U<<(short)(charMinusLowUInt32=(ushort)(a-'a')))&(charMinusLowUInt32-32))<0);}

C.IsReservedPattern(Int32) L0000: addecx,0xffffff9f L0003: movzxeax,cx L0006: movsxrdx,ax L000a: movecx,0xd4000000 L000f: shlxedx,ecx,edx L0014: addeax,0xffffffe0 L0017: andeax,edx L0019: shreax,0x1f L001c: ret

The code for this in the source generator is here:

runtime/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs

Lines 4857 to 4920 in 05e25ff

	// Next, handle sets where the high - low + 1 range is <= 32. In that case, we can emit
	// a branchless lookup in a uint that does not rely on loading any objects (e.g. the string-based
	// lookup we use later). This nicely handles common sets like [\t\r\n ].
	if(analysis.OnlyRanges&&(analysis.UpperBoundExclusiveIfOnlyRanges-analysis.LowerBoundInclusiveIfOnlyRanges)<=32)
	{
	additionalDeclarations.Add("uint charMinusLowUInt32;");
	// Create the 32-bit value with 1s at indices corresponding to every character in the set,
	// where the bit is computed to be the char value minus the lower bound starting from
	// most significant bit downwards.
	boolnegatedClass=RegexCharClass.IsNegated(charClass);
	uintbitmap=0;
	for(inti=analysis.LowerBoundInclusiveIfOnlyRanges;i<analysis.UpperBoundExclusiveIfOnlyRanges;i++)
	{
	if(RegexCharClass.CharInClass((char)i,charClass)^negatedClass)
	{
	bitmap|=1u<<(31-(i-analysis.LowerBoundInclusiveIfOnlyRanges));
	}
	}
	// To determine whether a character is in the set, we subtract the lowest char; this subtraction happens before the result is
	// zero-extended to uint, meaning that `charMinusLowUInt32` will always have upper 16 bits equal to 0.
	// We then left shift the constant with this offset, and apply a bitmask that has the highest
	// bit set (the sign bit) if and only if `chExpr` is in the [low, low + 32) range.
	// Then we only need to check whether this final result is less than 0: this will only be
	// the case if both `charMinusLowUInt32` was in fact the index of a set bit in the constant, and also
	// `chExpr` was in the allowed range (this ensures that false positive bit shifts are ignored).
	negate^=negatedClass;
	return$"((int)((0x{bitmap:X}U << (short)(charMinusLowUInt32 = (ushort)({chExpr} - {Literal((char)analysis.LowerBoundInclusiveIfOnlyRanges)}))) & (charMinusLowUInt32 - 32)) {(negate?">=":"<")} 0)";
	}
	// Next, handle sets where the high - low + 1 range is <= 64. As with the 32-bit case above, we can emit
	// a branchless lookup in a ulong that does not rely on loading any objects (e.g. the string-based lookup
	// we use later). Note that unlike RegexCompiler, the source generator doesn't know whether the code is going
	// to be run in a 32-bit or 64-bit process: in a 64-bit process, this is an optimization, but in a 32-bit process,
	// it's a deoptimization. In general we optimize for 64-bit perf, so this code remains; it complicates the code
	// too much to try to include both this and a fallback for the check. This, however, is why we do the 32-bit
	// version and check first, as that variant performs equally well on both 32-bit and 64-bit systems.
	if(analysis.OnlyRanges&&(analysis.UpperBoundExclusiveIfOnlyRanges-analysis.LowerBoundInclusiveIfOnlyRanges)<=64)
	{
	additionalDeclarations.Add("ulong charMinusLowUInt64;");
	// Create the 64-bit value with 1s at indices corresponding to every character in the set,
	// where the bit is computed to be the char value minus the lower bound starting from
	// most significant bit downwards.
	boolnegatedClass=RegexCharClass.IsNegated(charClass);
	ulongbitmap=0;
	for(inti=analysis.LowerBoundInclusiveIfOnlyRanges;i<analysis.UpperBoundExclusiveIfOnlyRanges;i++)
	{
	if(RegexCharClass.CharInClass((char)i,charClass)^negatedClass)
	{
	bitmap|=1ul<<(63-(i-analysis.LowerBoundInclusiveIfOnlyRanges));
	}
	}
	// To determine whether a character is in the set, we subtract the lowest char; this subtraction happens before
	// the result is zero-extended to uint, meaning that `charMinusLowUInt64` will always have upper 32 bits equal to 0.
	// We then left shift the constant with this offset, and apply a bitmask that has the highest bit set (the sign bit)
	// if and only if `chExpr` is in the [low, low + 64) range. Then we only need to check whether this final result is
	// less than 0: this will only be the case if both `charMinusLowUInt64` was in fact the index of a set bit in the constant,
	// and also `chExpr` was in the allowed range (this ensures that false positive bit shifts are ignored).
	negate^=negatedClass;
	return$"((long)((0x{bitmap:X}UL << (int)(charMinusLowUInt64 = (uint){chExpr} - {Literal((char)analysis.LowerBoundInclusiveIfOnlyRanges)})) & (charMinusLowUInt64 - 64)) {(negate?">=":"<")} 0)";
	}

[JulieLeeMSFT]

@JulieLeeMSFT you can set the code blocks to colorize, I modified your top post, basically just put C# or asm immediately after the first three `

Yes, I tested the code for non contiguous values and unsorted values. It works now. I am checking some other details.

[EgorBo]

It generates a branch-free version, e.g. for the set [abdf] that's being checked for in IsReservedPattern above, it emits the equivalent of this:

gcc emits closer to what Julie does: https://godbolt.org/z/rj1cPYz1a, basically, a single jump to filter out obviously wrong values and then a bit test (bt). No idea what's faster in reality. Although, it's not related to this PR directly, because what it does is it just emits a SWITCH opcode instead of series of comparisons

[JulieLeeMSFT]

Other patterns tested:

Pattern 2: handles character check. The first condition with GT_COMMA is skipped, and the rest of conditions are optimized.

if(c=='a'||c=='b'||c=='d'||c=='f'){returntrue;}returnfalse;

Pattern 3: handles unsorted patterns.

if(a==102||a==97||a==98||a==100)returntrue;returnfalse;

Pattern 4: handles min and max limit.

if(a==-9223372036854775806||a==-9223372036854775807||a==-9223372036854775804)returntrue;returnfalse;if(a==9223372036854775806||a==9223372036854775807||a==9223372036854775804)returntrue;returnfalse;

[JulieLeeMSFT]

Hi @EgorBo, this is ready for review. CC @dotnet/jit-contrib.

Asmdiff shows size regression, but they are all converted to a bit test instead of multiple compares and jumps.

• aspnet
  

• benchmarks:
  

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[EgorBo]

/azp run runtime-coreclr outerloop, runtime-coreclr pgo

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[EgorBo]

Slightly refactored the impl to make it smaller and extendable in future, so far it finds ~4000 contexts to improve.

Regressions also look like perf improvements because of less branches. We can increase minimal number of tests from 3 to 4 to get rid of size regressions but I guess we'd better do it based on microbenchmarks reports as the overall diffs aren't bad.

[teo-tsirpanis]

(changing milestone to 9.0.0, I assume it won't be backported)

[JulieLeeMSFT]

Verified that char, byte, and short types generate bt instruction.
cc @stephentoub.

• char type

[MethodImpl(MethodImplOptions.NoInlining)]staticboolTestRange(charc){if(c==' '||c=='\t'||c=='\r'||c=='\n')returntrue;returnfalse;}

IN0001: 000000movzxrcx,cxIN0002: 000003cmpecx,32IN0003: 000006je SHORT G_M24766_IG07IN0004: 000008cmpecx,13IN0005: 00000Bja SHORT G_M24766_IG05IN0006: 00000D moveax,0x19FFIN0007: 000012bteax,ecxIN0008: 000015jae SHORT G_M24766_IG07G_M24766_IG05: ; offs=0x000017, size=0x0002, bbWeight=0.50, PerfScore 0.12, gcrefRegs=0000 {}, byrefRegs=0000 {}, BB04 [0005], byrefIN0009: 000017xoreax,eaxIN000b: 000019retG_M24766_IG07: ; offs=0x00001A, size=0x0005, bbWeight=0.50, PerfScore 0.12, gcVars=0000000000000000 {}, gcrefRegs=0000 {}, byrefRegs=0000 {}, BB05 [0004], gcvars, byrefIN000a: 00001A moveax,1IN000c: 00001F ret

• short type

[MethodImpl(MethodImplOptions.NoInlining)]staticboolTestRange(shortc){if(c==' '||c=='\t'||c=='\r'||c=='\n')returntrue;returnfalse;}

IN0001: 000000movsxrcx,cxIN0002: 000004cmpecx,32IN0003: 000007je SHORT G_M54667_IG07IN0004: 000009cmpecx,13IN0005: 00000C ja SHORT G_M54667_IG05IN0006: 00000E moveax,0x19FFIN0007: 000013bteax,ecxIN0008: 000016jae SHORT G_M54667_IG07G_M54667_IG05: ; offs=0x000018, size=0x0002, bbWeight=0.50, PerfScore 0.12, gcrefRegs=0000 {}, byrefRegs=0000 {}, BB04 [0005], byrefIN0009: 000018xoreax,eaxIN000b: 00001A retG_M54667_IG07: ; offs=0x00001B, size=0x0005, bbWeight=0.50, PerfScore 0.12, gcVars=0000000000000000 {}, gcrefRegs=0000 {}, byrefRegs=0000 {}, BB05 [0004], gcvars, byrefIN000a: 00001Bmoveax,1IN000c: 000020ret

• byte type

[MethodImpl(MethodImplOptions.NoInlining)]staticboolTestRange(bytec){if(c==' '||c=='\t'||c=='\r'||c=='\n')returntrue;returnfalse;}

IN0001: 000000movzxrcx,clIN0002: 000003cmpecx,32IN0003: 000006je SHORT G_M29734_IG07IN0004: 000008cmpecx,13IN0005: 00000Bja SHORT G_M29734_IG05IN0006: 00000D moveax,0x19FFIN0007: 000012bteax,ecxIN0008: 000015jae SHORT G_M29734_IG07G_M29734_IG05: ; offs=0x000017, size=0x0002, bbWeight=0.50, PerfScore 0.12, gcrefRegs=0000 {}, byrefRegs=0000 {}, BB04 [0005], byrefIN0009: 000017xoreax,eaxIN000b: 000019retG_M29734_IG07: ; offs=0x00001A, size=0x0005, bbWeight=0.50, PerfScore 0.12, gcVars=0000000000000000 {}, gcrefRegs=0000 {}, byrefRegs=0000 {}, BB05 [0004], gcvars, byrefIN000a: 00001A moveax,1IN000c: 00001F ret

[JulieLeeMSFT]

If if range values are contiguous, it removes switch and simplifies to fall through to its unique successor with BBJ_ALWAYS. So, bt is not generated.
https://github.com/dotnet/runtime/blame/main/src/coreclr/jit/lower.cpp#L1018-L1034

[stephentoub]

Thanks, @JulieLeeMSFT. FYI, it fails to do so if you don't use if / return true;, e.g.

[MethodImpl(MethodImplOptions.NoInlining)]staticboolTestRange(charc)=>c==' '||c=='\t'||c=='\r'||c=='\n';

[JulieLeeMSFT]

I will check on it.

Thanks, @JulieLeeMSFT. FYI, it fails to do so if you don't use if / return true;, e.g.

[MethodImpl(MethodImplOptions.NoInlining)]staticboolTestRange(charc)=>c==' '||c=='\t'||c=='\r'||c=='\n';

[JulieLeeMSFT]

char range value creates a COMMA with STORE_LCL_VAR, which we skip for switch conversion. Then it leaves only 2 JTRUE nodes, one for \t and the other for \r, which is less than the minimum number of BBs we require for switch conversion. So, optimization does not kick in.
For char, if we add one more condition, !, the optimization kicks in and generates bt as shown below.
Verified that short and byte also work when adding one more value, !.
cc @stephentoub.

[MethodImpl(MethodImplOptions.NoInlining)]staticboolTestRange(charc)=>c==' '||c=='\t'||c=='\r'||c=='\n'||c=='!';

***** BB01 [0000]STMT00000 ( 0x000[E-] ... 0x003 )N008 ( 8,9) [000003]-A---------* JTRUE void$VN.VoidN007 ( 6,7) [000002]JA-----N--- \--* EQ int$101N005 ( 4,5) [000030]-A---------+--* COMMA int$100N003 ( 3,4) [000028] DA--------- | +--* STORE_LCL_VAR int V02 cse0 d:1$VN.VoidN002 ( 3,4) [000022]----------- | | \--* CAST int <- ushort <-int$100N001 ( 2,2) [000000]----------- | | \--* LCL_VAR int V00 arg0 u:1$80N004 ( 1,1) [000029]----------- | \--* LCL_VAR int V02 cse0 u:1$100N006 ( 1,1) [000001]----------- \--* CNS_INT int32$43------------ BB02 [0001][005..00A) -> BB06(0.5),BB03(0.5) (cond), preds={BB01} succs={BB03,BB06}***** BB02 [0001]STMT00002 ( 0x005[E-] ... 0x008 )N004 ( 5,5) [000009]-----------* JTRUE void$VN.VoidN003 ( 3,3) [000008] J------N--- \--* EQ int$102N001 ( 1,1) [000031]-----------+--* LCL_VAR int V02 cse0 u:1$100N002 ( 1,1) [000007]----------- \--* CNS_INT int9$44------------ BB03 [0002][00A..00F) -> BB06(0.5),BB04(0.5) (cond), preds={BB02} succs={BB04,BB06}***** BB03 [0002]STMT00003 ( 0x00A[E-] ... 0x00D )N004 ( 5,5) [000013]-----------* JTRUE void$VN.VoidN003 ( 3,3) [000012] J------N--- \--* EQ int$103N001 ( 1,1) [000032]-----------+--* LCL_VAR int V02 cse0 u:1$100N002 ( 1,1) [000011]----------- \--* CNS_INT int13$45------------ BB04 [0003][00F..014) -> BB06(0.5),BB05(0.5) (cond), preds={BB03} succs={BB05,BB06}***** BB04 [0003]STMT00004 ( 0x00F[E-] ... 0x012 )N004 ( 5,5) [000017]-----------* JTRUE void$VN.VoidN003 ( 3,3) [000016] J------N--- \--* EQ int$104N001 ( 1,1) [000033]-----------+--* LCL_VAR int V02 cse0 u:1$100N002 ( 1,1) [000015]----------- \--* CNS_INT int10$42------------ BB05 [0004][014..01A) (return), preds={BB04} succs={}***** BB05 [0004]STMT00005 ( 0x014[E-] ... 0x019 )N004 ( 7,4) [000021]-----------* RETURN int$VN.VoidN003 ( 6,3) [000020]----------- \--* EQ int$105N001 ( 1,1) [000034]-----------+--* LCL_VAR int V02 cse0 u:1$100N002 ( 1,1) [000019]----------- \--* CNS_INT int33$47------------ BB06 [0005][01A..01C) (return), preds={BB01,BB02,BB03,BB04} succs={}***** BB06 [0005]STMT00001 ( 0x01A[E-] ... 0x01B )N002 ( 2,2) [000005]-----------* RETURN int$VN.VoidN001 ( 1,1) [000026]----------- \--* CNS_INT int1$46

IN0001: 000000movzxrcx,cxIN0002: 000003cmpecx,32IN0003: 000006je SHORT G_M24766_IG05IN0004: 000008cmpecx,13IN0005: 00000Bja SHORT G_M24766_IG07IN0006: 00000D moveax,0x19FFIN0007: 000012bteax,ecxIN0008: 000015jb SHORT G_M24766_IG07G_M24766_IG05: IN0009: 000017moveax,1IN000d: 00001C retG_M24766_IG07: IN000a: 00001D cmpecx,33IN000b: 000020 sete alIN000c: 000023movzxrax,alIN000e: 000026ret

[JulieLeeMSFT]

BBs for

[MethodImpl(MethodImplOptions.NoInlining)]staticboolTestRange(charc){if(c==' '||c=='\t'||c=='\r'||c=='\n')returntrue;returnfalse;}

---------------------------------------------------------------------------------------------------------------------------------------------------------------------BBnum BBid ref try hnd preds weight [IL range][jump][EH region][flags]---------------------------------------------------------------------------------------------------------------------------------------------------------------------BB01 [0000]11[000..005)-> BB06(0.5),BB02(0.5) ( cond ) iBB02 [0001]1 BB01 0.50[005..00A)-> BB06(0.5),BB03(0.5) ( cond ) iBB03 [0002]1 BB02 0.50[00A..00F)-> BB06(0.5),BB04(0.5) ( cond ) iBB04 [0003]1 BB03 0.50[00F..014)-> BB05(0.5),BB06(0.5) ( cond ) iBB05 [0005]1 BB04 0.50[016..018) (return) iBB06 [0004]4 BB01,BB02,BB03,BB04 0.50[014..016) (return) i--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- BB01 [0000][000..005) -> BB06(0.5),BB02(0.5) (cond), preds={} succs={BB02,BB06}***** BB01 [0000]STMT00000 ( 0x000[E-] ... 0x003 )N008 ( 8,9) [000003]-A---------* JTRUE void$VN.VoidN007 ( 6,7) [000002]JA-----N--- \--* EQ int$101N005 ( 4,5) [000028]-A---------+--* COMMA int$100N003 ( 3,4) [000026] DA--------- | +--* STORE_LCL_VAR int V02 cse0 d:1$VN.VoidN002 ( 3,4) [000020]----------- | | \--* CAST int <- ushort <-int$100N001 ( 2,2) [000000]----------- | | \--* LCL_VAR int V00 arg0 u:1$80N004 ( 1,1) [000027]----------- | \--* LCL_VAR int V02 cse0 u:1$100N006 ( 1,1) [000001]----------- \--* CNS_INT int32$43------------ BB02 [0001][005..00A) -> BB06(0.5),BB03(0.5) (cond), preds={BB01} succs={BB03,BB06}***** BB02 [0001]STMT00002 ( 0x005[E-] ... 0x008 )N004 ( 5,5) [000009]-----------* JTRUE void$VN.VoidN003 ( 3,3) [000008] J------N--- \--* EQ int$102N001 ( 1,1) [000029]-----------+--* LCL_VAR int V02 cse0 u:1$100N002 ( 1,1) [000007]----------- \--* CNS_INT int9$44------------ BB03 [0002][00A..00F) -> BB06(0.5),BB04(0.5) (cond), preds={BB02} succs={BB04,BB06}***** BB03 [0002]STMT00003 ( 0x00A[E-] ... 0x00D )N004 ( 5,5) [000013]-----------* JTRUE void$VN.VoidN003 ( 3,3) [000012] J------N--- \--* EQ int$103N001 ( 1,1) [000030]-----------+--* LCL_VAR int V02 cse0 u:1$100N002 ( 1,1) [000011]----------- \--* CNS_INT int13$45------------ BB04 [0003][00F..014) -> BB05(0.5),BB06(0.5) (cond), preds={BB03} succs={BB06,BB05}***** BB04 [0003]STMT00004 ( 0x00F[E-] ... 0x012 )N004 ( 5,5) [000017]-----------* JTRUE void$VN.VoidN003 ( 3,3) [000016] N------N-U- \--* NE int$104N001 ( 1,1) [000031]-----------+--* LCL_VAR int V02 cse0 u:1$100N002 ( 1,1) [000015]----------- \--* CNS_INT int10$42------------ BB05 [0005][016..018) (return), preds={BB04} succs={}***** BB05 [0005]STMT00005 ( 0x016[E-] ... 0x017 )N002 ( 2,2) [000019]-----------* RETURN int$VN.VoidN001 ( 1,1) [000024]----------- \--* CNS_INT int0$40------------ BB06 [0004][014..016) (return), preds={BB01,BB02,BB03,BB04} succs={}***** BB06 [0004]STMT00001 ( 0x014[E-] ... 0x015 )N002 ( 2,2) [000005]-----------* RETURN int$VN.VoidN001 ( 1,1) [000025]----------- \--* CNS_INT int1$46

[EgorBo]

@JulieLeeMSFT regarding return <conditions> I have a prototype somewhere to normalize all return <condition> to if-else so all downstream phases don't have to care about it, so I presume we shouldn't care about BBJ_RETURN here for simplicity 🙂

The COMMA in the first condition is something we can handle I guess here

[JulieLeeMSFT]

@JulieLeeMSFT regarding return <conditions> I have a prototype somewhere to normalize all return <condition> to if-else so all downstream phases don't have to care about it, so I presume we shouldn't care about BBJ_RETURN here for simplicity 🙂

Very good! Let's check that in.

The COMMA in the first condition is something we can handle I guess here

Right, we didn't have time for that and left it for future work.