public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "cvs-commit at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug rtl-optimization/98212] [10/11 Regression] X86 unoptimal code for float equallity comparison followed by jump
Date: Thu, 10 Dec 2020 23:38:39 +0000	[thread overview]
Message-ID: <bug-98212-4-jv7TzXlKnd@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-98212-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98212

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:a5c05005499dd323296008fda4f414d8647adf0c

commit r11-5923-ga5c05005499dd323296008fda4f414d8647adf0c
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Dec 11 00:36:21 2020 +0100

    dojump: Fix up probabilities splitting in dojump.c comparison splitting
[PR98212]

    When compiling:
    void foo (void);
    void bar (float a, float b) { if (__builtin_expect (a != b, 1)) foo (); }
    void baz (float a, float b) { if (__builtin_expect (a == b, 1)) foo (); }
    void qux (float a, float b) { if (__builtin_expect (a != b, 0)) foo (); }
    void corge (float a, float b) { if (__builtin_expect (a == b, 0)) foo (); }
    on x86_64, we get (unimportant cruft removed):
    bar:    ucomiss %xmm1, %xmm0
            jp      .L4
            je      .L1
    .L4:    jmp     foo
    .L1:    ret
    baz:    ucomiss %xmm1, %xmm0
            jp      .L6
            jne     .L6
            jmp     foo
    .L6:    ret
    qux:    ucomiss %xmm1, %xmm0
            jp      .L13
            jne     .L13
            ret
    .L13:   jmp     foo
    corge:  ucomiss %xmm1, %xmm0
            jnp     .L18
    .L14:   ret
    .L18:   jne     .L14
            jmp     foo
    (note for bar and qux that changed with a patch I've posted earlier today).
    This is all reasonable, except the last function, the overall jump to
    the tail call is predicted unlikely (10%), so it is good jmp foo isn't on
    the straight line path, but NaNs are (or should be) considered very
unlikely
    in the programs, so IMHO the right code (and one emitted with the following
    patch) is:
    corge:  ucomiss %xmm1, %xmm0
            jp      .L14
            je      .L18
    .L14:   ret
    .L18:   jmp     foo

    Let's discuss the probabilities in the above testcase:
    for !and_them it looks all correct, so for
    bar we split
    if (a != b) goto t; // prob 90%
    goto f;
    into:
    if (a unord b) goto t; // first_prob = prob * cprob = 90% * 1% = 0.9%
    if (a ltgt b) goto t; // adjusted prob = (prob - first_prob) / (1 -
first_prob) = (90% - 0.9%) / (1 - 0.9%) = 89.909%
    and for qux we split
    if (a != b) goto t; // prob 10%
    goto f;
    into:
    if (a unord b) goto t; // first_prob = prob * cprob = 10% * 1% = 0.1%
    if (a ltgt b) goto t; // adjusted prob = (prob - first_prob) / (1 -
first_prob) = (10% - 0.1%) / (1 - 0.1%) = 9.910%
    Now, the and_them cases should be probability wise exactly the same
    if we swap the f and t labels, because baz
    if (a == b) goto t; // prob 90%
    goto f;
    is equivalent to:
    if (a != b) goto f; // prob 10%
    goto t;
    which is in qux.  This means we could expand baz as:
    if (a unord b) goto f; // 0.1%
    if (a ltgt b) goto f; // 9.910%
    goto t;
    But we don't expand it exactly that way, but instead (as the comment says)
    as:
    if (a ord b) ; else goto f; // first_prob as probability of ;
    if (a uneq b) goto t; // adjusted prob
    goto f;
    So, first_prob.invert () should be 0.1% and adjusted prob should be
    1 - 9.910%.
    Thus, the right thing is 4 inverts:
    prob = prob.invert (); // baz is equivalent to qux with swap(t, f) and thus
inverted original prob
    first_prob = prob.split (cprob.invert ()).invert ();
    // cprob.invert because by doing if (cond) ; else goto f; we effectively
invert the condition
    // the second invert because first_prob is probability of ; rather than
goto f
    prob = prob.invert (); // lastly because adjusted prob we want is
    // probability of goto t;, while the one from corresponding !and_them case
    // would be if (...) goto f; goto t;

    2020-12-11  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/98212
            * dojump.c (do_compare_rtx_and_jump): Change computation of
            first_prob for and_them.  Add comment explaining and_them case.

            * gcc.dg/predict-8.c: Adjust expected probability.

  parent reply	other threads:[~2020-12-10 23:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-09 10:07 [Bug rtl-optimization/98212] New: " denis.campredon at gmail dot com
2020-12-09 11:15 ` [Bug rtl-optimization/98212] " crazylht at gmail dot com
2020-12-09 12:46 ` [Bug rtl-optimization/98212] [10/11 Regression] " rguenth at gcc dot gnu.org
2020-12-09 12:50 ` ubizjak at gmail dot com
2020-12-09 14:43 ` jakub at gcc dot gnu.org
2020-12-09 16:54 ` jakub at gcc dot gnu.org
2020-12-09 19:10 ` jakub at gcc dot gnu.org
2020-12-09 19:43 ` jakub at gcc dot gnu.org
2020-12-10 11:04 ` cvs-commit at gcc dot gnu.org
2020-12-10 23:38 ` cvs-commit at gcc dot gnu.org [this message]
2020-12-10 23:40 ` [Bug rtl-optimization/98212] [10 " jakub at gcc dot gnu.org
2021-04-08 12:02 ` rguenth at gcc dot gnu.org
2022-06-28 10:42 ` jakub at gcc dot gnu.org
2023-07-07  9:19 ` rguenth at gcc dot gnu.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-98212-4-jv7TzXlKnd@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).