Package: coreutils;
Reported by: Paul Eggert <eggert <at> CS.UCLA.EDU>
Date: Mon, 26 Jul 2010 03:30:03 UTC
Severity: normal
Tags: patch
Done: Pádraig Brady <P <at> draigBrady.com>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Paul Eggert <eggert <at> CS.UCLA.EDU> To: 6728 <at> debbugs.gnu.org Subject: bug#6728: [PATCH] sort: omit 'restrict' in doubtful cases Date: Sun, 25 Jul 2010 20:29:45 -0700
Several of the uses of 'restrict' in sort.c are not needed,
and the one in the typedef might even be incorrect (though
it would be hard to come up with a test case proving this,
as it is compiler-dependent and would require a genius compiler).
I installed the following. It does not affect the code generated
on RHEL 5 x86-64 (gcc -O2), so any performance penalty of omitting
these 'restrict's is likely to be minor.
From f9f86499bf0d45df5e304ce014724d35f3386560 Mon Sep 17 00:00:00 2001
From: Paul R. Eggert <eggert <at> cs.ucla.edu>
Date: Sun, 25 Jul 2010 20:25:31 -0700
Subject: [PATCH] sort: omit 'restrict' in doubtful cases
* src/sort.c (lock_node, unlock_node, queue_destroy, queue_init):
(queue_pop):
Omit 'restrict'; it shouldn't help here, as these functions have just
one pointer parameter and don't access static storage.
(queue_insert, check_insert, update_parent): Omit 'restrict', as
the pointer types differ, and are not char * or unsigned char *,
and therefore can't alias.
(write_unique): Omit 'restrict', as the pointer types are all
read-only.
(merge_loop, sortlines): Omit 'restrict', as any performance
advantages are extremely unlikely and it's not worth cluttering
the code for that.
(struct thread_args): Omit 'restrict': this seems to be incorrect.
It's unlikely for 'restrict' to be correct inside a typedef.
---
src/sort.c | 36 +++++++++++++++++-------------------
1 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/src/sort.c b/src/sort.c
index ca1d95c..ea2720f 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -3119,7 +3119,7 @@ compare_nodes (void const *a, void const *b)
number of processors. */
static inline void
-lock_node (struct merge_node *restrict node)
+lock_node (struct merge_node *node)
{
pthread_spin_lock (node->lock);
}
@@ -3127,7 +3127,7 @@ lock_node (struct merge_node *restrict node)
/* Unlock a merge tree NODE. */
static inline void
-unlock_node (struct merge_node *restrict node)
+unlock_node (struct merge_node *node)
{
pthread_spin_unlock (node->lock);
}
@@ -3135,7 +3135,7 @@ unlock_node (struct merge_node *restrict node)
/* Destroy merge QUEUE. */
static inline void
-queue_destroy (struct merge_node_queue *restrict queue)
+queue_destroy (struct merge_node_queue *queue)
{
heap_free (queue->priority_queue);
pthread_cond_destroy (&queue->cond);
@@ -3148,7 +3148,7 @@ queue_destroy (struct merge_node_queue *restrict queue)
heap, RESERVE should be 2 * NTHREADS. */
static inline void
-queue_init (struct merge_node_queue *restrict queue, size_t reserve)
+queue_init (struct merge_node_queue *queue, size_t reserve)
{
queue->priority_queue = heap_alloc (compare_nodes, reserve);
pthread_mutex_init (&queue->mutex, NULL);
@@ -3159,8 +3159,7 @@ queue_init (struct merge_node_queue *restrict queue, size_t reserve)
or does not need to lock NODE. */
static inline void
-queue_insert (struct merge_node_queue *restrict queue,
- struct merge_node *restrict node)
+queue_insert (struct merge_node_queue *queue, struct merge_node *node)
{
pthread_mutex_lock (&queue->mutex);
heap_insert (queue->priority_queue, node);
@@ -3172,7 +3171,7 @@ queue_insert (struct merge_node_queue *restrict queue,
/* Pop NODE off priority QUEUE. Guarantee a non-null, spinlocked NODE. */
static inline struct merge_node *
-queue_pop (struct merge_node_queue *restrict queue)
+queue_pop (struct merge_node_queue *queue)
{
struct merge_node *node = NULL;
@@ -3199,8 +3198,7 @@ queue_pop (struct merge_node_queue *restrict queue)
thus is only appropriate for internal sort. */
static inline void
-write_unique (struct line const *restrict line, FILE *tfp,
- char const *temp_output)
+write_unique (struct line const *line, FILE *tfp, char const *temp_output)
{
static struct line const *saved = NULL;
@@ -3284,7 +3282,7 @@ mergelines_node (struct merge_node *restrict node, size_t total_lines,
/* Insert NODE into QUEUE if it passes insertion checks. */
static inline void
-check_insert (struct merge_node *node, struct merge_node_queue *restrict queue)
+check_insert (struct merge_node *node, struct merge_node_queue *queue)
{
size_t lo_avail = node->lo - node->end_lo;
size_t hi_avail = node->hi - node->end_hi;
@@ -3304,8 +3302,8 @@ check_insert (struct merge_node *node, struct merge_node_queue *restrict queue)
/* Update parent merge tree NODE. */
static inline void
-update_parent (struct merge_node *restrict node, size_t merged,
- struct merge_node_queue *restrict queue)
+update_parent (struct merge_node *node, size_t merged,
+ struct merge_node_queue *queue)
{
if (node->level > MERGE_ROOT)
{
@@ -3326,7 +3324,7 @@ update_parent (struct merge_node *restrict node, size_t merged,
some of those lines, until the MERGE_END node is popped. */
static void
-merge_loop (struct merge_node_queue *restrict queue,
+merge_loop (struct merge_node_queue *queue,
size_t total_lines, FILE *tfp, char const *temp_output)
{
while (1)
@@ -3352,8 +3350,8 @@ merge_loop (struct merge_node_queue *restrict queue,
static void sortlines (struct line *restrict, struct line *restrict,
unsigned long int, size_t,
- struct merge_node *restrict, bool,
- struct merge_node_queue *restrict,
+ struct merge_node *, bool,
+ struct merge_node_queue *,
FILE *, char const *);
/* Thread arguments for sortlines_thread. */
@@ -3364,9 +3362,9 @@ struct thread_args
struct line *dest;
unsigned long int nthreads;
size_t const total_lines;
- struct merge_node *const restrict parent;
+ struct merge_node *const parent;
bool lo_child;
- struct merge_node_queue *const restrict merge_queue;
+ struct merge_node_queue *const merge_queue;
FILE *tfp;
char const *output_temp;
};
@@ -3407,8 +3405,8 @@ sortlines_thread (void *data)
static void
sortlines (struct line *restrict lines, struct line *restrict dest,
unsigned long int nthreads, size_t total_lines,
- struct merge_node *restrict parent, bool lo_child,
- struct merge_node_queue *restrict merge_queue,
+ struct merge_node *parent, bool lo_child,
+ struct merge_node_queue *merge_queue,
FILE *tfp, char const *temp_output)
{
/* Create merge tree NODE. */
--
1.7.1
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.