From unknown Mon Jun 23 18:34:08 2025 X-Loop: help-debbugs@gnu.org Subject: bug#6728: [PATCH] sort: omit 'restrict' in doubtful cases Resent-From: Paul Eggert Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-coreutils@gnu.org Resent-Date: Mon, 26 Jul 2010 03:30:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 6728 X-GNU-PR-Package: coreutils X-GNU-PR-Keywords: patch To: 6728@debbugs.gnu.org X-Debbugs-Original-To: Bug Coreutils Received: via spool by submit@debbugs.gnu.org id=B.128011499123851 (code B ref -1); Mon, 26 Jul 2010 03:30:03 +0000 Received: (at submit) by debbugs.gnu.org; 26 Jul 2010 03:29:51 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OdENy-0006Ce-Sb for submit@debbugs.gnu.org; Sun, 25 Jul 2010 23:29:51 -0400 Received: from mail.gnu.org ([199.232.76.166] helo=mx10.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OdENw-0006CZ-OZ for submit@debbugs.gnu.org; Sun, 25 Jul 2010 23:29:49 -0400 Received: from lists.gnu.org ([199.232.76.165]:36775) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1OdEO1-0005ZC-1f for submit@debbugs.gnu.org; Sun, 25 Jul 2010 23:29:53 -0400 Received: from [140.186.70.92] (port=40050 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OdENz-0005Zm-AG for bug-coreutils@gnu.org; Sun, 25 Jul 2010 23:29:52 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,T_RP_MATCHES_RCVD autolearn=unavailable version=3.3.1 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OdENx-0003Tr-IC for bug-coreutils@gnu.org; Sun, 25 Jul 2010 23:29:51 -0400 Received: from kiwi.cs.ucla.edu ([131.179.128.19]:38749) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OdENx-0003TQ-23 for bug-coreutils@gnu.org; Sun, 25 Jul 2010 23:29:49 -0400 Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by kiwi.cs.ucla.edu (8.13.8+Sun/8.13.8/UCLACS-6.0) with ESMTP id o6Q3Tj6K024638 for ; Sun, 25 Jul 2010 20:29:46 -0700 (PDT) Message-ID: <4C4D0129.7030107@cs.ucla.edu> Date: Sun, 25 Jul 2010 20:29:45 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.10) Gecko/20100527 Thunderbird/3.0.5 MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: Solaris 10 (beta) X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Spam-Score: -5.1 (-----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -5.1 (-----) 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 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 From unknown Mon Jun 23 18:34:08 2025 MIME-Version: 1.0 X-Mailer: MIME-tools 5.427 (Entity 5.427) X-Loop: help-debbugs@gnu.org From: help-debbugs@gnu.org (GNU bug Tracking System) To: Paul Eggert Subject: bug#6728: closed (Re: bug#6728: [PATCH] sort: omit 'restrict' in doubtful cases) Message-ID: References: <4C4D599A.5060904@draigBrady.com> <4C4D0129.7030107@cs.ucla.edu> X-Gnu-PR-Message: they-closed 6728 X-Gnu-PR-Package: coreutils X-Gnu-PR-Keywords: patch Reply-To: 6728@debbugs.gnu.org Date: Mon, 26 Jul 2010 09:48:01 +0000 Content-Type: multipart/mixed; boundary="----------=_1280137681-2212-1" This is a multi-part message in MIME format... ------------=_1280137681-2212-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Your bug report #6728: [PATCH] sort: omit 'restrict' in doubtful cases which was filed against the coreutils package, has been closed. The explanation is attached below, along with your original report. If you require more details, please reply to 6728@debbugs.gnu.org. --=20 6728: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D6728 GNU Bug Tracking System Contact help-debbugs@gnu.org with problems ------------=_1280137681-2212-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at 6728-done) by debbugs.gnu.org; 26 Jul 2010 09:47:53 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OdKHo-0000ZV-BZ for submit@debbugs.gnu.org; Mon, 26 Jul 2010 05:47:52 -0400 Received: from mail1.slb.deg.dub.stisp.net ([84.203.253.98]) by debbugs.gnu.org with smtp (Exim 4.69) (envelope-from ) id 1OdKHm-0000ZQ-6P for 6728-done@debbugs.gnu.org; Mon, 26 Jul 2010 05:47:50 -0400 Received: (qmail 14208 invoked from network); 26 Jul 2010 09:47:54 -0000 Received: from unknown (HELO ?192.168.2.25?) (84.203.137.218) by mail1.slb.deg.dub.stisp.net with SMTP; 26 Jul 2010 09:47:54 -0000 Message-ID: <4C4D599A.5060904@draigBrady.com> Date: Mon, 26 Jul 2010 10:47:06 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 MIME-Version: 1.0 To: Paul Eggert Subject: Re: bug#6728: [PATCH] sort: omit 'restrict' in doubtful cases References: <4C4D0129.7030107@cs.ucla.edu> In-Reply-To: <4C4D0129.7030107@cs.ucla.edu> X-Enigmail-Version: 1.0.1 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Spam-Score: -2.7 (--) X-Debbugs-Envelope-To: 6728-done Cc: 6728-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.7 (--) On 26/07/10 04:29, Paul Eggert wrote: > Several of the uses of 'restrict' in sort.c are not needed, Right. Would only be useful for multiple pointers of the same type. Closing... ------------=_1280137681-2212-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at submit) by debbugs.gnu.org; 26 Jul 2010 03:29:51 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OdENy-0006Ce-Sb for submit@debbugs.gnu.org; Sun, 25 Jul 2010 23:29:51 -0400 Received: from mail.gnu.org ([199.232.76.166] helo=mx10.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OdENw-0006CZ-OZ for submit@debbugs.gnu.org; Sun, 25 Jul 2010 23:29:49 -0400 Received: from lists.gnu.org ([199.232.76.165]:36775) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1OdEO1-0005ZC-1f for submit@debbugs.gnu.org; Sun, 25 Jul 2010 23:29:53 -0400 Received: from [140.186.70.92] (port=40050 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OdENz-0005Zm-AG for bug-coreutils@gnu.org; Sun, 25 Jul 2010 23:29:52 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,T_RP_MATCHES_RCVD autolearn=unavailable version=3.3.1 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OdENx-0003Tr-IC for bug-coreutils@gnu.org; Sun, 25 Jul 2010 23:29:51 -0400 Received: from kiwi.cs.ucla.edu ([131.179.128.19]:38749) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OdENx-0003TQ-23 for bug-coreutils@gnu.org; Sun, 25 Jul 2010 23:29:49 -0400 Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by kiwi.cs.ucla.edu (8.13.8+Sun/8.13.8/UCLACS-6.0) with ESMTP id o6Q3Tj6K024638 for ; Sun, 25 Jul 2010 20:29:46 -0700 (PDT) Message-ID: <4C4D0129.7030107@cs.ucla.edu> Date: Sun, 25 Jul 2010 20:29:45 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.10) Gecko/20100527 Thunderbird/3.0.5 MIME-Version: 1.0 To: Bug Coreutils Subject: [PATCH] sort: omit 'restrict' in doubtful cases Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: Solaris 10 (beta) X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Spam-Score: -5.1 (-----) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -5.1 (-----) 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 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 ------------=_1280137681-2212-1--