public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Aldy Hernandez <aldyh@redhat.com>
To: GCC patches <gcc-patches@gcc.gnu.org>
Cc: Andrew MacLeod <amacleod@redhat.com>, Aldy Hernandez <aldyh@redhat.com>
Subject: [COMMITTED] [PR middle-end/106824]  Do not ICE when updating a NAN to a non-NAN.
Date: Mon,  5 Sep 2022 17:57:08 +0200	[thread overview]
Message-ID: <20220905155708.3420282-1-aldyh@redhat.com> (raw)

Updating a definite NAN to a definitely not NAN was an assertion
failure, but it turns out we can have such a scenario while attempting
to thread an impossible path.  This patch updates the range to
undefined.

What happens in the testcase is that we are trying to thread path that
starts like this:

    <bb 5> [local count: 81335936906]:
    SR.30_3 =  Nan;
    goto <bb 7>; [100.00%]

    <bb 7> [local count: 108447915740]:
    # SR.30_25 = PHI <SR.30_3(5), SR.30_12(6)>
    plus_33 = SR.30_25;
    w1$value__13 = f_distance$D2421$value__1;
    w2$value__14 = plus_33;
    if (w1$value__13 == w2$value__14)
      goto <bb 8>; [50.00%]
    else
      goto <bb 9>; [50.00%]

On the path, SR.30_25 is NAN, which ultimately makes w2$value__14 a NAN.

This means that the 7->8 is impossible on the path.

On the true arm (foperator_equal::op1_range) we are asserting that op1
(w1$value__13) is a !NAN because for the == conditional to succeed,
neither operand can be a NAN.  But...we know that operand 2 is a NAN.
This is an impossible scenario.

We are ICEing because frange::set_nan() sees the NAN and the desire to
set the NAN flag to NO.  The correct thing to do is to set the range
to undefined, which is basically unreachable, and will cause all the
right things to happen.  For that matter, the threader will see that
an UNDEFINED range was calculated in the path and abort trying to
investigate paths in that direction.

Tested on x86-64 Linux with regstrap as well as mpfr tests.

	PR middle-end/106824

gcc/ChangeLog:

	* value-range.cc (frange::set_nan): Set undefined when updating a
	NAN to a non-NAN.

gcc/testsuite/ChangeLog:

	* g++.dg/pr106824.C: New test.
---
 gcc/testsuite/g++.dg/pr106824.C | 76 +++++++++++++++++++++++++++++++++
 gcc/value-range.cc              | 12 +++++-
 2 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr106824.C

diff --git a/gcc/testsuite/g++.dg/pr106824.C b/gcc/testsuite/g++.dg/pr106824.C
new file mode 100644
index 00000000000..bd80be0dfaa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr106824.C
@@ -0,0 +1,76 @@
+// { dg-do compile }
+// { dg-options "-O2 -w -std=c++11" }
+
+using int32 = int;
+int ShortestPath_distance;
+struct FloatWeightTpl {
+  FloatWeightTpl(float f) : value_(f) {}
+  float Value() { return value_; }
+  float value_;
+};
+template <class T> bool operator!=(FloatWeightTpl w1, T w2) {
+  bool __trans_tmp_2;
+  FloatWeightTpl __trans_tmp_3 = w1;
+  __trans_tmp_2 = __trans_tmp_3.Value() == w2.Value();
+  return __trans_tmp_2;
+}
+struct TropicalWeightTpl : FloatWeightTpl {
+  TropicalWeightTpl(float f) : FloatWeightTpl(f) {}
+  static TropicalWeightTpl Zero();
+  static TropicalWeightTpl NoWeight() {
+    float __trans_tmp_5 = __builtin_nanf("");
+    return __trans_tmp_5;
+  }
+  bool Member() { return value_; }
+};
+TropicalWeightTpl Plus(TropicalWeightTpl w1, TropicalWeightTpl &w2) {
+  return w1.Member() || w2.Member() ? TropicalWeightTpl::NoWeight()          : w2.Value()               ? : w2;
+}
+TropicalWeightTpl Times();
+struct ArcTpl {
+  using Weight = TropicalWeightTpl;
+};
+template <class, class, class> struct ShortestPathOptions {
+  ShortestPathOptions(int, int, int32, bool, bool);
+};
+template <class Arc, class Queue, class ArcFilter>
+void SingleShortestPath(ShortestPathOptions<Arc, Queue, ArcFilter>) {
+  using Weight = typename Arc::Weight;
+  auto f_distance = Weight::Zero();
+  while (!0) {
+    TropicalWeightTpl __trans_tmp_1 = Times(),
+                      plus = Plus(f_distance, __trans_tmp_1);
+    if (f_distance != plus)
+      f_distance = plus;
+  }
+}
+template <class Arc, class Queue, class ArcFilter>
+void ShortestPath(int, int *, int *,
+                  ShortestPathOptions<Arc, Queue, ArcFilter> opts) {
+  SingleShortestPath(opts);
+}
+struct ShortestDistanceOptions {
+  float delta;
+};
+struct Trans_NS_script_ShortestPathOptions : ShortestDistanceOptions {
+  int32 nshortest;
+  bool unique;
+};
+namespace internal {
+template <class, class>
+void ShortestPath(int ifst, int *ofst, int *distance,
+                  Trans_NS_script_ShortestPathOptions opts) {
+  using ArcFilter = int;
+  ShortestPathOptions<ArcTpl, int, ArcFilter> sopts(opts.nshortest, opts.unique,
+                                                    false, opts.delta, 0);
+  ShortestPath(ifst, ofst, distance, sopts);
+}
+int ShortestPath_ifst;
+int ShortestPath_ofst;
+Trans_NS_script_ShortestPathOptions ShortestPath_opts;
+void ShortestPath() {
+  using StateId = int;
+  ShortestPath<ArcTpl, StateId>(ShortestPath_ifst, &ShortestPath_ofst,
+                                &ShortestPath_distance, ShortestPath_opts);
+}
+} // namespace internal
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 9c561415971..c3f668a811a 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -274,13 +274,21 @@ frange::set_nan (fp_prop::kind k)
 {
   if (k == fp_prop::YES)
     {
+      if (get_nan ().no_p ())
+	{
+	  set_undefined ();
+	  return;
+	}
       gcc_checking_assert (!undefined_p ());
       *this = frange_nan (m_type);
       return;
     }
 
-  // Setting NO on an obviously NAN range is nonsensical.
-  gcc_checking_assert (k != fp_prop::NO || !real_isnan (&m_min));
+  if (k == fp_prop::NO && get_nan ().yes_p ())
+    {
+      set_undefined ();
+      return;
+    }
 
   // Setting VARYING on an obviously NAN range is a no-op.
   if (k == fp_prop::VARYING && real_isnan (&m_min))
-- 
2.37.1


                 reply	other threads:[~2022-09-05 15:57 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20220905155708.3420282-1-aldyh@redhat.com \
    --to=aldyh@redhat.com \
    --cc=amacleod@redhat.com \
    --cc=gcc-patches@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).