public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "aldyh at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/55157] Missed VRP with != 0 and multiply
Date: Fri, 04 Nov 2022 11:22:47 +0000	[thread overview]
Message-ID: <bug-55157-4-ov0Ha1jUJ2@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-55157-4@http.gcc.gnu.org/bugzilla/>

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

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aldyh at gcc dot gnu.org,
                   |                            |amacleod at redhat dot com

--- Comment #4 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
We have a variety of ways of fixing this:
  1. Improve the range-op entry for MULT_EXPR.
  2. Handle nonzero bits in the relational operators.
  3. Reflect the nonzero bits in the actual range for trivial masks.

At evrp time we have:

=========== BB 2 ============
Imports: t_3(D)  
Exports: t_3(D)  g_4  
         g_4 : t_3(D)(I)  
t_3(D)  [irange] unsigned int VARYING
    <bb 2> :
    g_4 = t_3(D) * 16;
    if (g_4 == 0)
      goto <bb 3>; [INV]
    else
      goto <bb 4>; [INV]

2->3  (T) g_4 :         [irange] unsigned int [0, 0] NONZERO 0x0
2->4  (F) g_4 :         [irange] unsigned int [1, +INF]

Since this is unsigned, improving the multiply range-op entry could give a
range for g_4 of [0,0][16,+INF].  This would be enough to fold the other
conditional: if (g_4 <= 4).

The code for mult and div is legacy code inherited from legacy VRP.  I'm
assuming we didn't handle the above originally because legacy ranges couldn't
represent [0,0][16,+INF].  We can now, so we could improve here.

However, even without a multiplication improvement, the nonzero mask could help
here.  Though this mask is missing by evrp time (because mult doesn't handle it
(yet)), CCP2 does provide it and we can see it by vrp1 time:

        g_4: [irange] unsigned int [1, +INF] NONZERO 0xfffffff0

This should be enough to fold the second conditional: if (g_4 <= 4).  But alas,
the LE_EXPR range-op entry does not take into account nonzero bits.

The above mask of 0xfffffff0 is fancy talk for [0, 0][16, +INF] which we could
intersect with [1, +INF].  We could teach the LE_EXPR range-op entry about
nonzero bits.

However, perhaps instead of teaching all the relation operators about nonzero
bits, we could augment the range in irange::set_nonzero_bits().  Basically,
intersecting [0,0][16,+INF] with the [1, +INF] when setting the nonzero mask.

The patch below does this, but it does have a 3% penalty for VRP (though no
penalty to overall compilation).  I'm inclined to pursue this route, since it
makes nonzero mask optimization more pervasive across the board.

What do you think Andrew?

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index a855aaf626c..db6a3b7bcb6 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -2962,6 +2962,20 @@ irange::set_nonzero_bits (const wide_int_ref &bits)
   normalize_kind ();
   if (flag_checking)
     verify_range ();
+
+  // Reflect the mask as a simple range.  For example, a mask of
+  // 0xff00 could be represented as [0,0][0x100, 0xffff].
+  if (TYPE_UNSIGNED (type ()) && prec > 1)
+    {
+      gcc_checking_assert (nz != 0);
+      wide_int lb = wi::lshift (wi::one (prec), wi::ctz (nz));
+      wide_int ub = wi::lrshift (wi::minus_one (prec), wi::clz (nz));
+      int_range<2> r (type (), lb, ub);
+      int_range<2> zero;
+      zero.set_zero (type ());
+      r.union_ (zero);
+      intersect (r);
+    }
 }

 // Return the nonzero bitmask.  This will return the nonzero bits plus

  parent reply	other threads:[~2022-11-04 11:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-31 21:51 [Bug tree-optimization/55157] New: Missing VRP pinskia at gcc dot gnu.org
2021-12-25 23:35 ` [Bug tree-optimization/55157] Missed VRP with != 0 and multiply pinskia at gcc dot gnu.org
2022-11-04 11:22 ` aldyh at gcc dot gnu.org [this message]
2022-11-04 11:39 ` aldyh at gcc dot gnu.org
2022-11-04 13:40 ` amacleod at redhat dot com
2022-11-04 15:44 ` aldyh at gcc dot gnu.org
2022-11-04 15:46 ` aldyh at gcc dot gnu.org
2022-11-04 19:45 ` aldyh at gcc dot gnu.org
2022-11-04 19:48 ` aldyh at gcc dot gnu.org
2022-11-04 20:03 ` aldyh at gcc dot gnu.org
2022-11-05  9:04 ` aldyh at gcc dot gnu.org
2022-11-07 19:59 ` cvs-commit at gcc dot gnu.org
2022-11-07 20:02 ` aldyh at gcc dot gnu.org
2022-11-08 18:26 ` pinskia 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-55157-4-ov0Ha1jUJ2@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).