[4/5] Add pp_humanized_limit and pp_humanized_range

Message ID 1536949038-34114-5-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series
  • RFC: gimple-ssa-sprintf.c: a new approach (PR middle-end/77696)
Related show

Commit Message

David Malcolm Sept. 14, 2018, 6:17 p.m.
gcc/ChangeLog:
	* pretty-print.c (get_power_of_two): New function.
	(struct relative_to_power_of_2): New struct.
	(pp_humanized_limit): New function.
	(pp_humanized_range): New function.
	(selftest::assert_pp_humanized_limit): New function.
	(ASSERT_PP_HUMANIZED_LIMIT): New macro.
	(selftest::assert_pp_humanized_range): New function.
	(ASSERT_PP_HUMANIZED_RANGE): New macro.
	(selftest::test_get_power_of_two): New function.
	(selftest::test_humanized_printing): New function.
	(selftest::pretty_print_c_tests): Call them.
	* pretty-print.h (pp_humanized_limit): New function.
	(pp_humanized_range): New function.
---
 gcc/pretty-print.c | 181 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 gcc/pretty-print.h |   4 ++
 2 files changed, 185 insertions(+)

-- 
1.8.5.3

Comments

Martin Sebor Sept. 18, 2018, 3:38 p.m. | #1
On 09/14/2018 12:17 PM, David Malcolm wrote:
> gcc/ChangeLog:

> 	* pretty-print.c (get_power_of_two): New function.

> 	(struct relative_to_power_of_2): New struct.

> 	(pp_humanized_limit): New function.

> 	(pp_humanized_range): New function.

> 	(selftest::assert_pp_humanized_limit): New function.

> 	(ASSERT_PP_HUMANIZED_LIMIT): New macro.

> 	(selftest::assert_pp_humanized_range): New function.

> 	(ASSERT_PP_HUMANIZED_RANGE): New macro.

> 	(selftest::test_get_power_of_two): New function.

> 	(selftest::test_humanized_printing): New function.

> 	(selftest::pretty_print_c_tests): Call them.

> 	* pretty-print.h (pp_humanized_limit): New function.

> 	(pp_humanized_range): New function.

...
> +static void

> +test_humanized_printing ()

> +{

> +  ASSERT_PP_HUMANIZED_LIMIT ((1<<16) + 1, "(1<<16)+1");

> +  ASSERT_PP_HUMANIZED_LIMIT (100000, "100000");

> +  ASSERT_PP_HUMANIZED_LIMIT (1<<17, "1<<17");

> +  ASSERT_PP_HUMANIZED_LIMIT ((1<<17) - 1, "(1<<17)-1");

> +  ASSERT_PP_HUMANIZED_LIMIT ((1<<17) + 1, "(1<<17)+1");

> +  ASSERT_PP_HUMANIZED_LIMIT (4294967295, "(1<<32)-1");

> +

> +  ASSERT_PP_HUMANIZED_RANGE (0, 0, "0");

> +  ASSERT_PP_HUMANIZED_RANGE (0, 1, "0...1");

> +  ASSERT_PP_HUMANIZED_RANGE ((1<<16), (1<<17), "65536...1<<17");


I didn't comment on this aspect of the change yesterday: besides
making very large numbers easier for humans to understand, my hope
was to also help avoid data model dependencies in the test suite.

When testing on any given host it's easy to forget that a constant
may have a different value in a different data model and hardcode
a fixed number.  The test then fails for targets where the constant
has a different value.

As it is, the sprintf (and other) tests deal with the problem like
this:

   T (-1, "%*cX", INT_MAX,     '1'); /* { dg-warning "output of \[0-9\]+ 
bytes causes result to exceed .INT_MAX." } */

I.e., by accepting any sequence of digits where a large constant
like INT_MAX is expected.  This works fine but doesn't verify that
the value printed is correct.

One approach to dealing with this is to use manifest constants
in the diagnostic output that are independent of the data model,
like INT_MAX, SIZE_MAX, etc.

Using shift expressions instead doesn't solve this problem,
but even beyond that, I wouldn't consider shift expressions
like "(1 << 16) + 1" to be significantly more readable than
their value.  IMO, it overestimates the capacity of most
programmers, to do shift arithmetic in their heads ;-)

Do you see a problem with using the <limits.h> constants?

Martin

Patch

diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 7dd900b..7a2cd30 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -1799,6 +1799,107 @@  pp_end_quote (pretty_printer *pp, bool show_color)
   pp_string (pp, close_quote);
 }
 
+/* Get the largest power of two that is less that or equal to VAL.
+
+   FIXME: does this exist in stdlib (e.g. log2l in C99)?  */
+
+static unsigned
+get_power_of_two (unsigned HOST_WIDE_INT val)
+{
+  int power_of_two = 0;
+  while (val >= 2)
+    {
+      val >>= 1;
+      power_of_two++;
+    }
+  return power_of_two;
+}
+
+/* A way to express a number in terms of an offset from a power of two.  */
+
+struct relative_to_power_of_2
+{
+  relative_to_power_of_2 (unsigned HOST_WIDE_INT val)
+  {
+    m_shift = get_power_of_two (val);
+    m_offset = val - (1ul << m_shift);
+
+    if (m_shift > 1)
+      if (m_offset > (1l << (m_shift - 1)))
+	{
+	  m_shift++;
+	  m_offset = val - (1ul << m_shift);
+	}
+  }
+
+  void print (pretty_printer *pp) const
+  {
+    if (m_offset > 0)
+      pp_printf (pp, "(1<<%u)+%wd", m_shift, m_offset);
+    else if (m_offset < 0)
+      pp_printf (pp, "(1<<%u)-%wd", m_shift, -m_offset);
+    else
+      pp_printf (pp, "1<<%u", m_shift);
+  }
+
+  unsigned m_shift;
+  HOST_WIDE_INT m_offset;
+};
+
+/* Print VAL to PP, in a "humanized" way.
+   Small numbers are printed in decimal form (e.g. "42").
+   Large numbers close to a power of two are printed relative to a
+   power of two.  For example, 4294967295 is printed as "(1<<32)-1".  */
+
+void
+pp_humanized_limit (pretty_printer *pp, unsigned HOST_WIDE_INT val)
+{
+  /* For large numbers near a power of two, print them symbolically.
+     "65536" is about the most I can recognize by eye (dmalcolm),
+     1<<17 is where I can't tell at a glance if it is indeed 1<<17.  */
+  const unsigned threshold = 1 << 16;
+  if (val > threshold)
+    {
+      relative_to_power_of_2 rel2 (val);
+
+      /* How near to the large power of two should they be to be
+	 printed in terms of it?  */
+      const unsigned threshold_2 = 100;
+      if (abs (rel2.m_offset) <= threshold_2)
+	{
+	  rel2.print (pp);
+	  return;
+	}
+    }
+
+  /* Otherwise, just print the value as a decimal.  */
+  pp_printf (pp, "%wu", val);
+}
+
+/* Print the range MIN to MAX to PP, in a "humanized" way.
+   For example if MIN == MAX, just one number is printed.
+
+   Return true if both values are 1, false otherwise.
+   FIXME: how do plural forms work for *ranges*?  */
+
+bool
+pp_humanized_range (pretty_printer *pp, unsigned HOST_WIDE_INT min,
+		    unsigned HOST_WIDE_INT max)
+{
+  if (min == max)
+    {
+      pp_humanized_limit (pp, min);
+      return min == 1;
+    }
+  else
+    {
+      pp_humanized_limit (pp, min);
+      pp_string (pp, "...");
+      pp_humanized_limit (pp, max);
+      return false;
+    }
+}
+
 
 /* The string starting at P has LEN (at least 1) bytes left; if they
    start with a valid UTF-8 sequence, return the length of that
@@ -2211,6 +2312,84 @@  test_pp_format ()
 		    "problem with %qs at line %i", "bar", 10);
 }
 
+/* Implementation detail of ASSERT_PP_HUMANIZED_LIMIT.  */
+
+static void
+assert_pp_humanized_limit (const location &loc, unsigned HOST_WIDE_INT val,
+			   const char *expected)
+{
+  pretty_printer pp;
+  pp_humanized_limit (&pp, val);
+  ASSERT_STREQ_AT (loc, expected, pp_formatted_text (&pp));
+}
+
+/* Verify that pp_humanized_limit (pp, VAL) is EXPECTED.  */
+
+#define ASSERT_PP_HUMANIZED_LIMIT(VAL, EXPECTED)			\
+  SELFTEST_BEGIN_STMT							\
+    assert_pp_humanized_limit ((SELFTEST_LOCATION), (VAL), (EXPECTED)); \
+  SELFTEST_END_STMT
+
+/* Implementation detail of ASSERT_PP_HUMANIZED_RANGE.  */
+
+static void
+assert_pp_humanized_range (const location &loc, unsigned HOST_WIDE_INT min,
+			   unsigned HOST_WIDE_INT max, const char *expected)
+{
+  pretty_printer pp;
+  pp_humanized_range (&pp, min, max);
+  ASSERT_STREQ_AT (loc, expected, pp_formatted_text (&pp));
+}
+
+/* Verify that pp_humanized_range (pp, MIN, MAX) is EXPECTED.  */
+
+#define ASSERT_PP_HUMANIZED_RANGE(MIN, MAX, EXPECTED)			\
+  SELFTEST_BEGIN_STMT							\
+    assert_pp_humanized_range ((SELFTEST_LOCATION), (MIN), (MAX), (EXPECTED)); \
+  SELFTEST_END_STMT
+
+/* Verify that get_power_of_two works as expected.  */
+
+static void
+test_get_power_of_two ()
+{
+  ASSERT_EQ (get_power_of_two (0), 0);
+  ASSERT_EQ (get_power_of_two (1), 0);
+  ASSERT_EQ (get_power_of_two (2), 1);
+
+  ASSERT_EQ (get_power_of_two (7), 2);
+  ASSERT_EQ (get_power_of_two (8), 3);
+  ASSERT_EQ (get_power_of_two (9), 3);
+
+  ASSERT_EQ (get_power_of_two (65535), 15);
+  ASSERT_EQ (get_power_of_two (65536), 16);
+}
+
+/* Verify that pp_humanized_limit and pp_humanized_range work as expected.  */
+
+static void
+test_humanized_printing ()
+{
+  ASSERT_PP_HUMANIZED_LIMIT (0, "0");
+  ASSERT_PP_HUMANIZED_LIMIT (1, "1");
+  ASSERT_PP_HUMANIZED_LIMIT (2, "2");
+  ASSERT_PP_HUMANIZED_LIMIT (3, "3");
+  ASSERT_PP_HUMANIZED_LIMIT (4, "4");
+  ASSERT_PP_HUMANIZED_LIMIT (42, "42");
+  ASSERT_PP_HUMANIZED_LIMIT (256, "256");
+  ASSERT_PP_HUMANIZED_LIMIT (1<<16, "65536");
+  ASSERT_PP_HUMANIZED_LIMIT ((1<<16) + 1, "(1<<16)+1");
+  ASSERT_PP_HUMANIZED_LIMIT (100000, "100000");
+  ASSERT_PP_HUMANIZED_LIMIT (1<<17, "1<<17");
+  ASSERT_PP_HUMANIZED_LIMIT ((1<<17) - 1, "(1<<17)-1");
+  ASSERT_PP_HUMANIZED_LIMIT ((1<<17) + 1, "(1<<17)+1");
+  ASSERT_PP_HUMANIZED_LIMIT (4294967295, "(1<<32)-1");
+
+  ASSERT_PP_HUMANIZED_RANGE (0, 0, "0");
+  ASSERT_PP_HUMANIZED_RANGE (0, 1, "0...1");
+  ASSERT_PP_HUMANIZED_RANGE ((1<<16), (1<<17), "65536...1<<17");
+}
+
 /* Run all of the selftests within this file.  */
 
 void
@@ -2218,6 +2397,8 @@  pretty_print_c_tests ()
 {
   test_basic_printing ();
   test_pp_format ();
+  test_get_power_of_two ();
+  test_humanized_printing ();
 }
 
 } // namespace selftest
diff --git a/gcc/pretty-print.h b/gcc/pretty-print.h
index 2decc51..e4a8c97 100644
--- a/gcc/pretty-print.h
+++ b/gcc/pretty-print.h
@@ -416,4 +416,8 @@  pp_wide_integer (pretty_printer *pp, HOST_WIDE_INT i)
 template<unsigned int N, typename T>
 void pp_wide_integer (pretty_printer *pp, const poly_int_pod<N, T> &);
 
+extern void pp_humanized_limit (pretty_printer *, unsigned HOST_WIDE_INT);
+extern bool pp_humanized_range (pretty_printer *, unsigned HOST_WIDE_INT,
+				unsigned HOST_WIDE_INT);
+
 #endif /* GCC_PRETTY_PRINT_H */