Summary: | [12/13/14/15 regression]: sum += (x>128 ? x : 0) puts the cmov on the critical path (at -O2) | ||
---|---|---|---|
Product: | gcc | Reporter: | Peter Cordes <pcordes> |
Component: | target | Assignee: | Not yet assigned to anyone <unassigned> |
Status: | NEW --- | ||
Severity: | normal | CC: | dimhen, hp, jeffreyalaw, law, roger, tavianator |
Priority: | P2 | Keywords: | missed-optimization |
Version: | 8.0 | ||
Target Milestone: | 12.5 | ||
See Also: | https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80520 | ||
Host: | Target: | x86_64-*-*, i?86-*-* | |
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: | 2023-05-30 00:00:00 | |
Bug Depends on: | |||
Bug Blocks: | 85559 | ||
Attachments: | Reproducer (micro benchmark) |
Description
Peter Cordes
2017-10-22 22:56:48 UTC
Created attachment 42437 [details] Reproducer (micro benchmark) Confirmed, I'm attaching micro-benchmark that I run on my Haswell machine. In time between GCC 6.x and current trunk we first improved performance in r239414 from 1.392704s -> 1.228814s. Then we significantly regressed in r242832 from 1.209454s -> 1.929302s. This is another case where PRE does the right thing on a local basis, but the result isn't well handled later in the pipeline. Prior to PRE the key blocks look like: <bb3> [ ... ] if (_4 > 127) goto <bb 4>; [50.00%] else goto <bb 5>; [50.00%] <bb 4> [local count: 531502203]: <bb 5> [local count: 1063004407]: # _17 = PHI <_4(4), 0(3)> iftmp.0_8 = (long long int) _17; sum_13 = iftmp.0_8 + sum_19; Not surprisingly PRE realizes it can reduce the total expressions evaluated on the path 3->5 by moving the arithmetic into bb4: <bb 3> [ ... ] if (_4 > 127) goto <bb 4>; [50.00%] else goto <bb 5>; [50.00%] <bb 4> [local count: 531502203]: _22 = (long long int) _4; _24 = sum_19 + _22; <bb 5> [local count: 1063004407]: # _17 = PHI <_4(4), 0(3)> # prephitmp_25 = PHI <_24(4), sum_19(3)> We keep that form through the rest of the gimple optimizers and into RTL where it turns into the problematic cmov during the first if-conversion pass. Compiling with -fno-tree-pre gives code that I believe is equivalent to gcc-6.x. I haven't looked deeply into this, but prior to Richi's PRE patch in r239414 we had a CMOV instruction. So if anything, Richi's patch removes the problematic instruction. It is with Bernd's patch tweaking costs in r242832 that we get the CMOV again. Could Bernd or someone else take a look at the cost patch to see if it's something simple that backend experts can tackle? GCC 7.3 is being released, adjusting target milestone. Aldy, It's less about whether or not there is a cmov (I get one with and without PRE), but more about what part of the loop we're using the cmov for. Consider what we get in the .optimized dump at -O3 correct. But the target *assembly* we want is what you'd get with -O3 -fno-tree-pre. ie, PRE is making it harder for us to generate the cmov we want. The GCC 7 branch is being closed, re-targeting to GCC 8.4. GCC 8.4.0 has been released, adjusting target milestone. GCC 8 branch is being closed. GCC 9.4 is being released, retargeting bugs to GCC 9.5. GCC 9 branch is being closed GCC 10.4 is being released, retargeting bugs to GCC 10.5. GCC 10 branch is being closed. I don't know if this could help here but combine does produce: Trying 37, 34 -> 38: 37: flags:CCGC=cmp(r82:SI,0x7f) REG_DEAD r82:SI 34: {r92:DI=r87:DI+r89:DI;clobber flags:CC;} REG_DEAD r89:DI REG_UNUSED flags:CC 38: r87:DI={(flags:CCGC>0)?r92:DI:r87:DI} REG_DEAD r92:DI REG_DEAD flags:CCGC Failed to match this instruction: (set (reg/v:DI 87 [ <retval> ]) (plus:DI (mult:DI (gt:DI (reg:SI 82 [ _4 ]) (const_int 127 [0x7f])) (reg:DI 89 [ _4 ])) (reg/v:DI 87 [ <retval> ]))) I don't know if we could catch this in the backend and split this into 3 instructions if that would do the right thing ... A better approach might be to to try and create COND_EXPRs for the conditional move in the gimple code. The biggest problem I see with that is the gimple->rtl converters aren't great at creating efficient code on targets without conditional moves. Meaning that we could well end up improving x86, but making several other targets worse. I know this because I was recently poking at a similar problem. We expressed a conditional move of 0, C as a multiply of a boolean by C in gimple. It really should just have been a COND_EXPR, but when we generate that form targets without good conditional move expanders will end up recreating branchy code :( GCC 11 branch is being closed. |