This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Remove a pointless global var
On Thu, 1 Aug 2019, Alexander Monakov wrote:
> 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.
Hey - that's nicely small!
> > 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;
Oh, of course.
I think a patch like this is OK.
Thanks,
Richard.
> 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. */
>
--
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)