From: Jonathan Nieder <jrnieder@gmail.com>
To: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Cc: Michael Haggerty <mhagger@alum.mit.edu>,
Junio C Hamano <gitster@pobox.com>,
Alex Riesen <raa.lkml@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH] for_each_string_list_item(): behave correctly for empty list
Date: Tue, 19 Sep 2017 19:30:08 -0700 [thread overview]
Message-ID: <20170920023008.GB126984@aiede.mtv.corp.google.com> (raw)
In-Reply-To: <b03c7b09-853f-a2ed-f73e-7d946c90cedb@gmail.com>
Hi,
Kaartic Sivaraam wrote:
> On Saturday 16 September 2017 09:36 AM, Michael Haggerty wrote:
>> Jonathan Nieder wrote:
>>> Does the following alternate fix work? I think I prefer it because
>>> it doesn't require introducing a new global. [...]
>>> #define for_each_string_list_item(item,list) \
>>> - for (item = (list)->items; item < (list)->items + (list)->nr; ++item)
>>> + for (item = (list)->items; \
>>> + (list)->items && item < (list)->items + (list)->nr; \
>>> + ++item)
>>
>> This is the possibility that I was referring to as "add[ing] overhead to
>> each iteration of the loop". I'd rather not add an extra test-and-branch
>> to every iteration of a loop in which `list->items` is *not* NULL, which
>> your solution appears to do. Or are compilers routinely able to optimize
>> the check out?
>
> I t seems at least 'gcc' is able to optimize this out even with a -O1
> and 'clang' optimizes this out with a -O2. Taking a sneak peek at
> the 'Makefile' shows that our default is -O2.
>
> For a proof, see https://godbolt.org/g/CPt73L
From that link:
for ( ;valid_int && *valid_int < 10; (*valid_int)++) {
printf("Valid instance");
}
Both gcc and clang are able to optimize out the 'valid_int &&' because
it is dereferenced on the RHS of the &&.
For comparison, 'item < (list)->items + (list)->nr' does not
dereference (list)->items. So that optimization doesn't apply here.
A smart compiler could be able to take advantage of there being no
object pointed to by a null pointer, which means
item < (list)->items + (list)->nr
is always false when (list)->items is NULL, which in turn makes a
'(list)->items &&' test redundant. But a quick test with gcc 4.8.4
-O2 finds that at least this compiler does not contain such an
optimization. The overhead Michael Haggerty mentioned is real.
Thanks and hope that helps,
Jonathan
next prev parent reply other threads:[~2017-09-20 2:30 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-15 16:00 [PATCH] for_each_string_list_item(): behave correctly for empty list Michael Haggerty
2017-09-15 18:43 ` Jonathan Nieder
2017-09-16 4:06 ` Michael Haggerty
2017-09-16 11:51 ` SZEDER Gábor
2017-09-17 10:19 ` Michael Haggerty
2017-09-19 14:38 ` Kaartic Sivaraam
2017-09-20 1:38 ` Junio C Hamano
2017-09-20 1:43 ` Jonathan Nieder
2017-09-20 5:14 ` Junio C Hamano
2017-09-20 2:30 ` Jonathan Nieder [this message]
2017-09-20 3:54 ` Junio C Hamano
2017-09-20 5:27 ` [PATCH v2] for_each_string_list_item: avoid undefined behavior " Jonathan Nieder
2017-09-20 5:40 ` Junio C Hamano
2017-09-20 7:00 ` Michael Haggerty
2017-09-20 7:40 ` Kaartic Sivaraam
2017-09-20 12:22 ` [PATCH v2] doc: camelCase the config variables to improve readability Kaartic Sivaraam
2017-09-20 16:28 ` [PATCH v2] for_each_string_list_item: avoid undefined behavior for empty list Andreas Schwab
2017-09-20 17:31 ` Jonathan Nieder
2017-09-20 21:51 ` Andreas Schwab
2017-09-21 1:12 ` Junio C Hamano
2017-09-21 15:39 ` Andreas Schwab
2017-09-20 7:35 ` [PATCH] for_each_string_list_item(): behave correctly " Kaartic Sivaraam
2017-09-17 0:59 ` Junio C Hamano
2017-09-17 10:24 ` Michael Haggerty
2017-09-18 0:37 ` Junio C Hamano
2017-09-19 0:08 ` Stefan Beller
2017-09-19 6:51 ` Michael Haggerty
2017-09-19 13:38 ` SZEDER Gábor
2017-09-19 13:45 ` SZEDER Gábor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170920023008.GB126984@aiede.mtv.corp.google.com \
--to=jrnieder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kaarticsivaraam91196@gmail.com \
--cc=mhagger@alum.mit.edu \
--cc=raa.lkml@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).