This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Remove a pointless global var


On Thu, 1 Aug 2019, Richard Biener wrote:

> Ugh.  Yeah, it's probably some inlining parameter.  OTOH I assumed
> the inliner would always inline into a simple wrapper if that makes
> the wrapped function unused - of course if we can't make that
> function static then it cannot know...  it looks like we put the 
> functions into anonmous namespaces though!
> 
> Does that fix the inlining?

No (the problem appeared with functions that are static and single-use).

> I disliked duplicating the code, not duplicating the instantiation.
> Using templates sounds fine if it doesn't end up being too ugly.

Hm.  Then please have a look at the alternative patch below.

> Hmm, it works to silence the fnptr to void * casting but not the other
> way around:
> 
> void foo ();
> int main()
> {
>   void (*p)() = foo;
>   void *data = (void *)p;
>   p = (void *)data;

data is already a 'void *', so the cast does nothing; you have to spell out
the type of the LHS fully:

  p = (void (*)())data;

diff --git a/gcc/sort.cc b/gcc/sort.cc
index 3e9c032c462..4e7915efcb6 100644
--- a/gcc/sort.cc
+++ b/gcc/sort.cc
@@ -51,15 +51,34 @@ typedef int cmp_fn (const void *, const void *);
 /* Structure holding read-mostly (read-only in netsort) context.  */
 struct sort_ctx
 {
-  cmp_fn *cmp; // pointer to comparator
+  cmp_fn *cmp_;// pointer to comparator
   char   *out; // output buffer
   size_t n;    // number of elements
   size_t size; // element size
   size_t nlim; // limit for network sort
+  int cmp (const void *a, const void *b)
+  {
+    return cmp_ (a, b);
+  }
+};
+
+struct sort_r_ctx
+{
+  void          *data;
+  sort_r_cmp_fn *cmp_;
+  char   *out;
+  size_t n;
+  size_t size;
+  size_t nlim;
+  int cmp (const void *a, const void *b)
+  {
+    return cmp_ (a, b, data);
+  }
 };
 
 /* Helper for netsort. Permute, possibly in-place, 2 or 3 elements,
    placing E0 to C->OUT, E1 to C->OUT + C->SIZE, and so on.  */
+template<typename sort_ctx>
 static void
 reorder23 (sort_ctx *c, char *e0, char *e1, char *e2)
 {
@@ -90,6 +109,7 @@ do {                                                     \
 }
 
 /* Like reorder23, but permute 4 or 5 elements.  */
+template<typename sort_ctx>
 static void
 reorder45 (sort_ctx *c, char *e0, char *e1, char *e2, char *e3, char *e4)
 {
@@ -127,21 +147,23 @@ do {                                                     \
    Return E0^E1 if E0 compares less than E1, zero otherwise.
    This is noinline to avoid code growth and confine invocation
    to a single call site, assisting indirect branch prediction.  */
+template<typename sort_ctx>
 noinline static intptr_t
-cmp1 (char *e0, char *e1, cmp_fn *cmp)
+cmp1 (char *e0, char *e1, sort_ctx *c)
 {
   intptr_t x = (intptr_t)e0 ^ (intptr_t)e1;
-  return x & (cmp (e0, e1) >> 31);
+  return x & (c->cmp (e0, e1) >> 31);
 }
 
 /* Execute network sort on 2 to 5 elements from IN, placing them into C->OUT.
    IN may be equal to C->OUT, in which case elements are sorted in place.  */
+template<typename sort_ctx>
 static void
 netsort (char *in, sort_ctx *c)
 {
 #define CMP(e0, e1)                   \
 do {                                  \
-  intptr_t x = cmp1 (e1, e0, c->cmp); \
+  intptr_t x = cmp1 (e1, e0, c);      \
   e0 = (char *)((intptr_t)e0 ^ x);    \
   e1 = (char *)((intptr_t)e1 ^ x);    \
 } while (0)
@@ -176,6 +198,7 @@ do {                                  \
 /* Execute merge sort on N elements from IN, placing them into OUT,
    using TMP as temporary storage if IN is equal to OUT.
    This is a stable sort if netsort is used only for 2 or 3 elements.  */
+template<typename sort_ctx>
 static void
 mergesort (char *in, sort_ctx *c, size_t n, char *out, char *tmp)
 {
@@ -217,6 +240,12 @@ do {                                            \
   memcpy (out, l, r - out);
 }
 
+static int
+cmp2to3 (const void *a, const void *b, void *c)
+{
+  return ((int (*)(const void *, const void *))c) (a, b);
+}
+
 void
 gcc_qsort (void *vbase, size_t n, size_t size, cmp_fn *cmp)
 {
@@ -235,7 +264,25 @@ gcc_qsort (void *vbase, size_t n, size_t size, cmp_fn *cmp)
   if (buf != scratch)
     free (buf);
 #if CHECKING_P
-  qsort_chk (vbase, n, size, cmp);
+  qsort_chk (vbase, n, size, cmp2to3, (void*)cmp);
+#endif
+}
+
+void
+gcc_qsort (void *vbase, size_t n, size_t size, sort_r_cmp_fn *cmp, void *data)
+{
+  if (n < 2)
+    return;
+  char *base = (char *)vbase;
+  sort_r_ctx c = {data, cmp, base, n, size, 5};
+  long long scratch[32];
+  size_t bufsz = (n / 2) * size;
+  void *buf = bufsz <= sizeof scratch ? scratch : xmalloc (bufsz);
+  mergesort (base, &c, n, base, (char *)buf);
+  if (buf != scratch)
+    free (buf);
+#if CHECKING_P
+  qsort_chk (vbase, n, size, cmp, data);
 #endif
 }
 
diff --git a/gcc/system.h b/gcc/system.h
index d04f8fd3360..b217b3cb2a5 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -1200,7 +1200,9 @@ helper_const_non_const_cast (const char *p)
 /* GCC qsort API-compatible functions: except in release-checking compilers,
    redirect 4-argument qsort calls to gcc_qsort; keep 1-argument invocations
    corresponding to vec::qsort (cmp): they use C qsort internally anyway.  */
-void qsort_chk (void *, size_t, size_t, int (*)(const void *, const void *));
+typedef int sort_r_cmp_fn (const void *, const void *, void *);
+void qsort_chk (void *, size_t, size_t, sort_r_cmp_fn *, void *);
+void gcc_sort_r (void *, size_t, size_t, sort_r_cmp_fn *, void *);
 void gcc_qsort (void *, size_t, size_t, int (*)(const void *, const void *));
 void gcc_stablesort (void *, size_t, size_t,
 		     int (*)(const void *, const void *));
diff --git a/gcc/vec.c b/gcc/vec.c
index cbd2db010f9..cadece45a2a 100644
--- a/gcc/vec.c
+++ b/gcc/vec.c
@@ -192,21 +192,23 @@ dump_vec_loc_statistics (void)
 ATTRIBUTE_NORETURN ATTRIBUTE_COLD
 static void
 qsort_chk_error (const void *p1, const void *p2, const void *p3,
-		 int (*cmp) (const void *, const void *))
+		 sort_r_cmp_fn *cmp, void *data)
 {
   if (!p3)
     {
-      int r1 = cmp (p1, p2), r2 = cmp (p2, p1);
+      int r1 = cmp (p1, p2, data), r2 = cmp (p2, p1, data);
       error ("qsort comparator not anti-commutative: %d, %d", r1, r2);
     }
   else if (p1 == p2)
     {
-      int r = cmp (p1, p3);
+      int r = cmp (p1, p3, data);
       error ("qsort comparator non-negative on sorted output: %d", r);
     }
   else
     {
-      int r1 = cmp (p1, p2), r2 = cmp (p2, p3), r3 = cmp (p1, p3);
+      int r1 = cmp (p1, p2, data);
+      int r2 = cmp (p2, p3, data);
+      int r3 = cmp (p1, p3, data);
       error ("qsort comparator not transitive: %d, %d, %d", r1, r2, r3);
     }
   internal_error ("qsort checking failed");
@@ -215,8 +217,7 @@ qsort_chk_error (const void *p1, const void *p2, const void *p3,
 /* Verify anti-symmetry and transitivity for comparator CMP on sorted array
    of N SIZE-sized elements pointed to by BASE.  */
 void
-qsort_chk (void *base, size_t n, size_t size,
-	   int (*cmp)(const void *, const void *))
+qsort_chk (void *base, size_t n, size_t size, sort_r_cmp_fn *cmp, void *data)
 {
 #if 0
 #define LIM(n) (n)
@@ -225,9 +226,9 @@ qsort_chk (void *base, size_t n, size_t size,
 #define LIM(n) ((n) <= 16 ? (n) : 12 + floor_log2 (n))
 #endif
 #define ELT(i) ((const char *) base + (i) * size)
-#define CMP(i, j) cmp (ELT (i), ELT (j))
-#define ERR2(i, j) qsort_chk_error (ELT (i), ELT (j), NULL, cmp)
-#define ERR3(i, j, k) qsort_chk_error (ELT (i), ELT (j), ELT (k), cmp)
+#define CMP(i, j) cmp (ELT (i), ELT (j), data)
+#define ERR2(i, j) qsort_chk_error (ELT (i), ELT (j), NULL, cmp, data)
+#define ERR3(i, j, k) qsort_chk_error (ELT (i), ELT (j), ELT (k), cmp, data)
   size_t i1, i2, i, j;
   /* This outer loop iterates over maximum spans [i1, i2) such that
      elements within each span compare equal to each other.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]