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 tree-optimization/109154] [13 regression] jump threading de-optimizes nested floating point comparisons
Date: Sat, 15 Apr 2023 10:10:02 +0000	[thread overview]
Message-ID: <bug-109154-4-9o6DkrmGp5@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-109154-4@http.gcc.gnu.org/bugzilla/>

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

--- Comment #52 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:de0ee9d14165eebb3d31c84e98260c05c3b33acb

commit r13-7192-gde0ee9d14165eebb3d31c84e98260c05c3b33acb
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Apr 15 12:08:45 2023 +0200

    if-conv: Small improvement for expansion of complex PHIs [PR109154]

    The following patch is just a dumb improvement, gets rid of 2 unnecessary
    instructions on both the PR's original testcase and on the two reduced
ones,
    both on -mcpu=neoverse-v1 and -mavx512f.

    The thing is, if we have args_len (args_len >= 2) unique PHI arguments,
    we need only args_len - 1 COND_EXPRs to expand the PHI, because first
    COND_EXPR can merge 2 unique arguments and all the following ones merge
    another unique argument with the previously merged arguments,
    while the code for mysterious reasons was always emitting args_len
    COND_EXPRs, where the first COND_EXPR merged the first and second unique
    arguments, the second COND_EXPR merged the second unique argument with
    result of merging the first and second unique arguments and the rest was
    already expectable, nth COND_EXPR for n > 2 merged the nth unique argument
    with result of merging the previous unique arguments.
    Now, in my understanding, the bb_predicate for bb's predecessor need to
    form a disjunct set which together creates the successor's bb_predicate,
    so I don't see why we'd need to check all the bb_predicates, if we check
    all but one then when all those other ones are false the last bb_predicate
    is necessarily true.  Given that the code attempts to sort argument with
    most occurrences (so likely most complex combined predicate) last, I chose
    not to test that last argument's predicate.
    So e.g. on the testcase from comment 47 in the PR:
    void
    foo (int *f, int d, int e)
    {
      for (int i = 0; i < 1024; i++)
        {
          int a = f[i];
          int t;
          if (a < 0)
            t = 1;
          else if (a < e)
            t = 1 - a * d;
          else
            t = 0;
          f[i] = t;
        }
    }
    we used to emit:
      _7 = a_10 < 0;
      _21 = a_10 >= 0;
      _22 = a_10 < e_11(D);
      _23 = _21 & _22;
      _26 = a_10 >= e_11(D);
      _27 = _21 & _26;
      _ifc__42 = _7 ? 1 : t_13;
      _ifc__43 = _23 ? t_13 : _ifc__42;
      t_6 = _27 ? 0 : _ifc__43;
    while the following patch changes it to:
      _7 = a_10 < 0;
      _21 = a_10 >= 0;
      _22 = a_10 < e_11(D);
      _23 = _21 & _22;
      _ifc__42 = _23 ? t_13 : 0;
      t_6 = _7 ? 1 : _ifc__42;
    which I believe should be sufficient for a PHI <1, t_13, 0>.

    I've gathered some statistics and on x86_64-linux and i686-linux
    bootstraps/regtests, this code triggers:
         92 4 4
        112 2 4
        141 3 4
       4046 3 3
    (where 2nd number is args_len and 3rd argument EDGE_COUNT (bb->preds)
    and first argument count of those from sort | uniq -c | sort -n).
    In all these cases the patch should squeze one extra COND_EXPR and
    its associated predicate (the latter only if it wasn't used elsewhere).

    Incrementally, I think we should try to perform some analysis on which
    predicates depend on inverses of other predicates and if possible try
    to sort the arguments better and omit testing unnecessary predicates.
    So essentially for the above testcase deconstruct it back to:
      _7 = a_10 < 0;
      _22 = a_10 < e_11(D);
      _ifc__42 = _22 ? t_13 : 0;
      t_6 = _7 ? 1 : _ifc__42;
    which is like what this patch produces, but with the & a_10 >= 0 part
    removed, because the last predicate is a_10 < 0 and so testing a_10 >= 0
    on what appears on the false branch doesn't make sense.
    But I'm afraid that will take more work than is doable in stage4 right now.

    2023-04-15  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/109154
            * tree-if-conv.cc (predicate_scalar_phi): For complex PHIs, emit
just
            args_len - 1 COND_EXPRs rather than args_len.  Formatting fix.

  parent reply	other threads:[~2023-04-15 10:10 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-16 11:57 [Bug tree-optimization/109154] New: [13 regression] aarch64 -mcpu=neoverse-v1 microbude performance regression pgodbole at nvidia dot com
2023-03-16 13:11 ` [Bug tree-optimization/109154] " tnfchris at gcc dot gnu.org
2023-03-16 14:58 ` [Bug target/109154] " rguenth at gcc dot gnu.org
2023-03-16 17:03 ` tnfchris at gcc dot gnu.org
2023-03-16 17:03 ` [Bug target/109154] [13 regression] jump threading with de-optimizes nested floating point comparisons tnfchris at gcc dot gnu.org
2023-03-22 10:20 ` [Bug tree-optimization/109154] [13 regression] jump threading " aldyh at gcc dot gnu.org
2023-03-22 10:29 ` avieira at gcc dot gnu.org
2023-03-22 12:22 ` rguenth at gcc dot gnu.org
2023-03-22 12:42 ` rguenth at gcc dot gnu.org
2023-03-22 13:11 ` aldyh at gcc dot gnu.org
2023-03-22 14:00 ` amacleod at redhat dot com
2023-03-22 14:39 ` aldyh at gcc dot gnu.org
2023-03-27  8:09 ` rguenth at gcc dot gnu.org
2023-03-27  9:30 ` jakub at gcc dot gnu.org
2023-03-27  9:42 ` aldyh at gcc dot gnu.org
2023-03-27  9:44 ` jakub at gcc dot gnu.org
2023-03-27 10:18 ` rguenther at suse dot de
2023-03-27 10:40 ` jakub at gcc dot gnu.org
2023-03-27 10:44 ` jakub at gcc dot gnu.org
2023-03-27 10:54 ` rguenth at gcc dot gnu.org
2023-03-27 10:56 ` jakub at gcc dot gnu.org
2023-03-27 10:59 ` jakub at gcc dot gnu.org
2023-03-27 17:07 ` jakub at gcc dot gnu.org
2023-03-28  8:33 ` rguenth at gcc dot gnu.org
2023-03-28  9:01 ` cvs-commit at gcc dot gnu.org
2023-03-28 10:07 ` tnfchris at gcc dot gnu.org
2023-03-28 10:08 ` tnfchris at gcc dot gnu.org
2023-03-28 12:18 ` jakub at gcc dot gnu.org
2023-03-28 12:25 ` rguenth at gcc dot gnu.org
2023-03-28 12:42 ` rguenth at gcc dot gnu.org
2023-03-28 13:19 ` rguenth at gcc dot gnu.org
2023-03-28 13:44 ` jakub at gcc dot gnu.org
2023-03-28 13:52 ` jakub at gcc dot gnu.org
2023-03-28 15:31 ` amacleod at redhat dot com
2023-03-28 15:40 ` jakub at gcc dot gnu.org
2023-03-28 15:53 ` amacleod at redhat dot com
2023-03-28 15:58 ` jakub at gcc dot gnu.org
2023-03-28 16:42 ` amacleod at redhat dot com
2023-03-28 21:12 ` amacleod at redhat dot com
2023-03-29  6:33 ` cvs-commit at gcc dot gnu.org
2023-03-29  6:38 ` rguenth at gcc dot gnu.org
2023-03-29 22:41 ` amacleod at redhat dot com
2023-03-30 18:17 ` cvs-commit at gcc dot gnu.org
2023-04-05  9:28 ` tnfchris at gcc dot gnu.org
2023-04-05  9:34 ` ktkachov at gcc dot gnu.org
2023-04-11  9:36 ` rguenth at gcc dot gnu.org
2023-04-13 16:54 ` jakub at gcc dot gnu.org
2023-04-13 17:25 ` rguenther at suse dot de
2023-04-13 17:29 ` jakub at gcc dot gnu.org
2023-04-14 18:10 ` jakub at gcc dot gnu.org
2023-04-14 18:14 ` jakub at gcc dot gnu.org
2023-04-14 18:22 ` jakub at gcc dot gnu.org
2023-04-14 19:09 ` jakub at gcc dot gnu.org
2023-04-15 10:10 ` cvs-commit at gcc dot gnu.org [this message]
2023-04-17 11:07 ` jakub at gcc dot gnu.org
2023-04-25 18:32 ` [Bug tree-optimization/109154] [13/14 " tnfchris at gcc dot gnu.org
2023-04-25 18:34 ` jakub at gcc dot gnu.org
2023-04-26  6:58 ` rguenth at gcc dot gnu.org
2023-04-26  9:43 ` tnfchris at gcc dot gnu.org
2023-04-26 10:07 ` jakub at gcc dot gnu.org
2023-07-07 18:10 ` tnfchris at gcc dot gnu.org
2023-07-10  7:15 ` rguenth at gcc dot gnu.org
2023-07-10 10:33 ` tnfchris at gcc dot gnu.org
2023-07-10 10:46 ` rguenth at gcc dot gnu.org
2023-07-10 11:02 ` tnfchris at gcc dot gnu.org
2023-07-10 11:27 ` rguenth at gcc dot gnu.org
2023-07-10 11:49 ` tnfchris at gcc dot gnu.org
2023-07-14 10:22 ` cvs-commit at gcc dot gnu.org
2023-07-14 10:22 ` cvs-commit at gcc dot gnu.org
2023-07-27  9:25 ` rguenth at gcc dot gnu.org
2023-10-02 10:53 ` cvs-commit at gcc dot gnu.org
2023-10-18  8:54 ` cvs-commit at gcc dot gnu.org
2023-10-18  8:54 ` cvs-commit at gcc dot gnu.org
2023-10-18  8:54 ` cvs-commit at gcc dot gnu.org
2023-10-18  8:55 ` cvs-commit at gcc dot gnu.org
2023-11-09 14:20 ` cvs-commit at gcc dot gnu.org
2023-11-09 14:20 ` cvs-commit at gcc dot gnu.org
2023-11-09 14:20 ` cvs-commit at gcc dot gnu.org
2023-11-09 14:20 ` cvs-commit at gcc dot gnu.org
2023-11-09 14:20 ` cvs-commit at gcc dot gnu.org
2023-11-09 14:20 ` cvs-commit at gcc dot gnu.org
2023-11-09 14:20 ` cvs-commit at gcc dot gnu.org
2023-11-09 14:20 ` cvs-commit at gcc dot gnu.org
2023-11-09 14:25 ` [Bug tree-optimization/109154] [13 " tnfchris 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-109154-4-9o6DkrmGp5@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).