<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Issue</th>
<td>
<a href=https://github.com/llvm/llvm-project/issues/199893>199893</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>
[X86] Missed optimization: integer-derived no-NaN fact not used for ordered fcmp lowering
</td>
</tr>
<tr>
<th>Labels</th>
<td>
new issue
</td>
</tr>
<tr>
<th>Assignees</th>
<td>
</td>
</tr>
<tr>
<th>Reporter</th>
<td>
134ARG
</td>
</tr>
</table>
<pre>
When looking at SPEC CPU 2017 with LLVM, I found a possible missed optimization in `benchspec/CPU/538.imagick_r/src/magick/gem.c`, function `ConvertRGBToHSB`, which could be extracted and compiled alone as:
```c
typedef unsigned short Quantum;
static const double QuantumScale = 1.0/65535.0;
void ConvertRGBToHSB(Quantum red, Quantum green, Quantum blue,
double *hue, double *saturation, double *brightness) {
double b, delta, g, max, min, r;
*hue=0.0; *saturation=0.0; *brightness=0.0;
r=(double) red; g=(double) green; b=(double) blue;
min=r < g ? r : g; if (b < min) min=b;
max=r > g ? r : g; if (b > max) max=b;
if (max == 0.0) return;
delta=max-min;
*saturation=delta/max;
*brightness=QuantumScale*max;
if (delta == 0.0) return;
if (r == max) *hue=(g-b)/delta;
else if (g == max) *hue=2.0+(b-r)/delta;
else *hue=4.0+(r-g)/delta;
*hue/=6.0;
if (*hue < 0.0) *hue+=1.0;
}
```
The LLVM IR and x86 asm with O3 are like: https://godbolt.org/z/11o6zjsaK.
The relevant source line is:
```C
if (delta == 0.0) return;
```
In the generated LLVM IR this becomes:
```LLVM IR
%23 = fcmp oeq double %20, 0.000000e+00, !dbg !66
```
and X86 lowers it as:
```assembly
ucomisd xmm3, xmm5
jne .LBB0_2
jnp .LBB0_8
```
Here `delta` is derived from `unsigned short` inputs by `uitofp`, min/max selection, and `delta = max - min`. Therefore `r`, `g`, `b`, `min`, `max`, and `delta` are finite and never NaN. The compare therefore has a no-NaN operand fact in this context.
However, this fact is not exposed on the final fcmp, so X86 conservatively lowers the ordered equality test with an unordered/parity branch. If I manually mark the compare as:
```LLVM IR
%23 = fcmp nnan oeq double %20, 0.000000e+00
```
then X86 already emits the simpler branch shape with the unordered/parity arm removed: https://godbolt.org/z/d7h9vc4Wv, which becomes:
```assembly
ucomisd xmm3, xmm5
je .LBB0_8
```
So this does not seem to require a new X86 lowering rule. The missing piece appears to be preserving or exposing the no-NaN fact from the integer-derived min/max/delta chain to the final compare.
A slightly modified example shows that this is not only a one-branch effect. If the same `delta == 0.0` compare result has multiple users:
```c
typedef unsigned short Quantum;
static const double QuantumScale = 1.0 / 65535.0;
int hsb_multi(Quantum red, Quantum green, Quantum blue,
double *sat, double *bright, int *a, int *b, int *v) {
double r = (double)red;
double g = (double)green;
double bl = (double)blue;
double min = r < g ? r : g;
if (bl < min) min = bl;
double max = r > g ? r : g;
if (bl > max) max = bl;
if (max == 0.0) return 0;
double delta = max - min;
*sat = delta / max;
*bright = QuantumScale * max;
int c = (delta == 0.0);
// add multiple usages of the comparison result
if (c) *a = 10; else *a = 20;
*v = c ? 123 : 456;
if (c) *b = 30; else *b = 40;
return c ? 1 : 2;
}
```
The resulting LLVM IR and x86 assembly for current LLVM are like: https://godbolt.org/z/fd547rMjG.
Again, delta is never NaN for the same source-level reason. In current LLVM, without the nnan fact on the compare, X86 materializes ordered equality using the parity flag:
```
ucomisd
setnp
sete
and
...
cmovne ...
cmovne ...
cmovne ...
```
and then uses that result for the downstream conditional moves.
If I add nnan only to the same fcmp: https://godbolt.org/z/r3o9eaWTh, the parity handling disappears, and the downstream uses are driven by the simpler equality/non-equality condition:
```
ucomisd
...
cmove ...
cmove ...
setne
cmove ...
```
</pre>
<img width="1" height="1" alt="" src="http://email.email.llvm.org/o/eJy8WEtz2zjy_zTwpUsqCrReBx1kK8q4_kn-2SSzmVsKJJskEhBQAFCW8-m3GiAl6pFMZg-rcllEo9FvdP8o4ZysNOKKTR_YdHMnWl8bu5qk9-sPr-8yU7ysPteoQRnzTeoKhIeP7189wuP7P4Enkzk8S1_Dmzf_fsv4IzxBaVpdgICdcU5mCqGRzmEBZudlI38IL40GqYHNkgx1Xrsd5oxvH9__yfh2mi7GshGVzL99sYxvnaW9SGB8W2EzztksIU1lq_Mgi82SR6P3aP2H1w-fzB8fHzqO51rmNeSmVQVkCHjwVuQeCxC6gNw0O6looYxGEI6la5aEv1kS_3KWrP3LDgssodUhSgW42lgP_2qF9m3D0geWrJ0XXuaQG-08FKYlpzuGj7lQCCzdwGScML6dTafpdJzEc3sjC7g0nS-6o2CxIC_6ZWUR9ZCQqRYZf2TJulfK-LoOtAHBCd_aEPRzemZlVXuNzjG-BDYng6DfzwIvKi_ooaJ_jTiELxnk2OgB9CrTTRK8utA4JA8U9uQgwbJ0w_giaiZbyO_0AapLegxA-gDZ5U6IRCeODEw3Flj6CBWwdAv0vCZxDyBLYHyRhc3gybLjz47HxaE7_upXx1_FeCw7_uPxyNGIA-Wc0k6OBp98a3XHFQObbhpxGAX1x1Ceha6L_zaoOLKchXFYZYyvT5zRjiDhV5Z0fLbn6Zw6JpXxRTXKGF8yvu2sjudQOewOVz87zEkhFXQ2sj8TcWS-75ntqLrB3Ff2lqWb2al2ogVxMyS1c7Jnf2DpZtKzs_nm4oLH5acaQ_uCpw-hMxwWMxCuiX3t_1MQFkHJb0hlUHu_C52Cb6kdmSIzyo-NrRjf_mB8O5mY2Y-vTvzf-CTbosK90B6caW1OsjSCvNFv6Cr_XuIuPHjS4GuECjVaQQ2ud8fX0kGGuWmw19ef7FiIxKc8DT2qzJsdGPx-ahNTHjppMk7Ch0KaBArjkyKr6Gs2uzaIwvjXYgbKPKN1IP3N_iqcwyZTLyxZt7lppCvg0DQpiT80zZQl668aYfzm4SH5wsNqB_SJlMW12j_QIo2DWDqzBKSDAq3cYwGlNQ3tnTfywKR3rXeQvYRt6U256wZIaBF0_8ChwrxvouRdrwW6yodR4J4lY_hUo8XSRFNsJ4rNkur0mJ0e46l-IQ7dYqiDjKQiLKWWHsOWxj1aeCfeBXVhmBGHP6quhQMB2ozeiXdgdmjpVClyT6M3lEVutMeDH0MXO_NMMkl32I68DrTxgIedCSM81lkptVChWIjbmZBqmn5o98LLPaqXPvPEbmyBFgvA761Q0r-AR-fj7RIaWt3tM77dCUv7mRU6r8fwVMITNEK3QqkXaIT9FuT1zt4qqZ9WtdZC_05pX9WUJ-xDDgplURQvgI300TEnm51C29kLrhY7jH7R7g3HhKWp3pg9Tbi_bybFvF7u8_vP-xOYOb_L_-wuIfz69nw0MfOFwZh2h9iAN2Dxeysp4KDx-XSvCQzaVmEsQUJ5RNlJzBHEboeC8m8Iee0sUm3QtrGxmuiZotRVaKi2cEWJKLXHCu2ov7vHe9iPBchrQXVsBvXYlUXXeNfgFA1KKhxTyFJSAR4EJYxu_jNlUPjocFflRqsXEGA0jrqMYlli7kMhhnyLBs8uft-dZ8mxKC26Vvlw_ZpWeUn6Wof2fwAvgfEtnAFMmifaQ-2yL8GY_w5cnn3OgOUtREk0Usr4Wgyes8Hz_gbiDBAEhqguwsABuq2uWHpEeAZd1RXbCR4OGRt6B0k3cBsqDuBFkDjEi-FYpm6IjLgPbsPHS5FDDHkt8tdIEpJr7bfm0Rm0DHsdF9_CLVwZWM6Li69PnNEy7SE_BvkaqZyEUlsDURTDqyAqdGDKQSeXzuju3gw8zzscFz2ahLeIHjJGGk8G5u8DKQ8hn4TGv4b76ew87L3MLDCnZzIj7f70WhLj3EkM8vjf48joBjW3a0QZGzSUxkLeWovaR6Z_gi_LYno_t2-_vu7H9roS8Y0sJoJaWQ8NgqZj34rgc6RwjwosCmf0GJ70mSlhzkhfm9bH5kxDM7TmbvJ3XY74aAw0wqOVQskflNPLMd8em3w3_kolqus2GKLdzazw7NDrXf-E4UHouDUej8N33pg9QcO4PF-dL28A0zDRW4fdCOg6dh-swjxr5y2KhjpuIQn0CQU0tF03WwIuobKOoILGRjeJQqQDMPqNXNrULFF8_lRHzHUMUy10oSh0hXTdHO0R4YWBwQkqn4LmpCYIO8QlfSYY32qjR8fEHP26eCW4ysQwoviTFWULr-hDmXfFKi2W6VLc4Woyny8Xs-l0MbmrV1k5X-J8wsViKtJUiEIsRLHkKZZiuVhO-Z1c8YTPkimfJ_NkxifjHBeL5f1sWmb3eV7Mkd0n2AipxkrtGwrtnXSuxdVkuVws0zslMlQu_LDFOWGXsMs4Z9PNnV3RoVHWVo7dJ0o6705ivPQq_CL212LGpht4e_0bFqX4EqgM0QxBipYOUWX1lyNg0R493bVWrS6qRPq6zca5aRjfkjXd12hnzVfMPePb4IOjd83o5H7F_xMAAP__NFH0oQ">