compiling: int IsHTMLWhitespace(int aChar) { return aChar == 0x0009 || aChar == 0x000A || aChar == 0x000C || aChar == 0x000D || aChar == 0x0020; } q(int a,int b, int c) { return IsHTMLWhitespace (a) && IsHTMLWhitespace (b) && IsHTMLWhitespace (c); } we do quite funny things by ipa-splitting IsHTMLWhitespace: IsHTMLWhitespace (int aChar) { unsigned int aChar.1_1; unsigned int _2; _Bool _3; _Bool _4; _Bool _5; int iftmp.0_6; int iftmp.0_8; <bb 2> [100.00%]: aChar.1_1 = (unsigned int) aChar_7(D); _2 = aChar.1_1 + 4294967287; _3 = _2 <= 1; _4 = aChar_7(D) == 12; _5 = _3 | _4; if (_5 != 0) goto <bb 4>; [46.00%] else goto <bb 3>; [54.00%] <bb 3> [54.00%]: iftmp.0_8 = IsHTMLWhitespace.part.0 (aChar_7(D)); <bb 4> [100.00%]: # iftmp.0_6 = PHI <1(2), iftmp.0_8(3)> return iftmp.0_6; } this is partly caused by the fact that we are not able to optimize early the sequence of compares to shift. In Firefox later we fail to inline the functions and produce some terrible code which slows down rerf-reftest/dep-check-1.html Firefox benchmark
With what options? I'm getting 3 bit tests both with -O2 and -O3, both when using C and C++. And get that also if I rewrite the function to use a switch instead.
> With what options? I'm getting 3 bit tests both with -O2 and -O3, both when > using C and C++. And get that also if I rewrite the function to use a switch > instead. -O2 -flto and then look into release_ssa dump. In Firefox we then fail to inline things back together. I am debugging that but already producing that at firstplace is bad.
The only pass that can do about this (at least right now) is reassoc (both 1 and 2), which is too late for inlining. So, either teach fnsplit not to separate multiple if comparisons of the same variable against constants, or schedule reasoc or just the maybe_optimize_range_tests part thereof in some early pass.
> The only pass that can do about this (at least right now) is reassoc (both 1 > and 2), which is too late for inlining. So, either teach fnsplit not to > separate multiple if comparisons of the same variable against constants, or > schedule reasoc or just the maybe_optimize_range_tests part thereof in some > early pass. Yep, I also found out about reassoc. Teaching fnsplit to pattern match this is just a partial solution - we would still miscalculate size of function body for functions like this (which indeed look quite common). I will experiment with early reassoc. I kind of debugged what happens later. Because code is compiled with -O2 and growth gets positive for both inlines and functions are not inline, we won't inline.
You could also add match.pd rules merging stuff pair-wise... How's this a regression btw?
> You could also add match.pd rules merging stuff pair-wise... > > How's this a regression btw? I was comparing with GCC 6 build where inlining apparently happened. I believe inlining happens again - I am waiting for benchmarks to finish. Honza
Just for the record, when rewriting the code with switch: int IsHTMLWhitespace(int aChar) { switch (aChar) { case 0x0009: case 0x000A: case 0x000C: case 0x000D: case 0x0020: return 1; default: return 0; } } one gets a nice bit-test eventually: <bb 2> [local count: 1073741824]: _4 = (unsigned int) aChar_2(D); if (_4 > 32) goto <bb 4>; [20.00%] else goto <bb 3>; [80.00%] <bb 3> : _6 = 1 << _4; _7 = _6 & 4294981120; _8 = _7 != 0; _5 = (int) _8; <bb 4> [local count: 1073741824]: # _1 = PHI <_5(3), 0(2)> return _1;
Honza can you please create mozilla upstream bug where we can recommend to use switch instead of series of if conditions?
If using a switch is better than a series of tests against constants, would it make sense for the compiler to spot this case, and automatically convert the conditions to a switch?
(In reply to David Malcolm from comment #9) > If using a switch is better than a series of tests against constants, would > it make sense for the compiler to spot this case, and automatically convert > the conditions to a switch? Yes, we've got a PR for it somewhere..
(In reply to Martin Liška from comment #10) > (In reply to David Malcolm from comment #9) > > If using a switch is better than a series of tests against constants, would > > it make sense for the compiler to spot this case, and automatically convert > > the conditions to a switch? > > Yes, we've got a PR for it somewhere.. bug 46935? or bug 14799?