From: "Philip Oakley" <philipoakley@iee.org>
To: "Stefan Beller" <stefanbeller@googlemail.com>,
<git@vger.kernel.org>, <gitster@pobox.com>
Cc: <sunshine@sunshineco.com>, <nico@fluxnic.net>
Subject: Re: [PATCH] create_delta_index: simplify condition always evaluating to true
Date: Sat, 17 Aug 2013 07:58:39 +0100 [thread overview]
Message-ID: <E5FB18126E7F42D79E085F3924AC9E07@PhilipOakley> (raw)
In-Reply-To: 1376688157-8374-1-git-send-email-stefanbeller@googlemail.com
From: "Stefan Beller" <stefanbeller@googlemail.com>
> The code sequence ' (1u << i) < hsize && i < 31 ' is a multi step
> process, whose first step requires that 'i' is already less that 31,
> otherwise the result (1u << i) is undefined (and 'undef_val < hsize'
> can therefore be assumed to be 'false'), and so the later test i < 31
> can always be optimized away as dead code ('i' is already less than
> 31,
> or the short circuit 'and' applies).
>
> So we need to get rid of that code. One way would be to exchange the
> order of the conditions, so the expression 'i < 31 && (1u << i) <
> hsize'
> would remove that optimized unstable code already.
>
> However when checking the previous lines in that function, we can
> deduce
> that 'hsize' must always be smaller than (1u<<31), since 506049c7df2c6
> (fix >4GiB source delta assertion failure), because 'entries' is
> capped at an upper bound of 0xfffffffeU, so 'hsize' contains a maximum
> value of 0x3fffffff, which is smaller than (1u<<31), so the value of
> 'i' will never be larger than 31 and we can remove that condition
> entirely.
>
> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
> Acked-by: Nicolas Pitre <nico@fluxnic.net>
> ---
Acked-by: Philip Oakley philipoakley@iee.org
If there are concens that future upgrades may make the (i < 31) test a
necessity that it should be added before the &&. If the compiler is able
to determine it's dead code in that case then that's OK, at least it
would be future proof. Then again it's probably not likely in the near
future.
>
> Philip,
> thanks for the wording of your mail. I get quickly derailed from the
> warnings of the STACK tool and write the other commit messages than
> I ought to write. I think the wording of your mail nails it pretty
> good, so I put it in the commit message as well.
>
> Stefan
>
>
> diff-delta.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/diff-delta.c b/diff-delta.c
> index 93385e1..3797ce6 100644
> --- a/diff-delta.c
> +++ b/diff-delta.c
> @@ -155,7 +155,7 @@ struct delta_index * create_delta_index(const void
> *buf, unsigned long bufsize)
> entries = 0xfffffffeU / RABIN_WINDOW;
> }
> hsize = entries / 4;
> - for (i = 4; (1u << i) < hsize && i < 31; i++);
> + for (i = 4; (1u << i) < hsize; i++);
> hsize = 1 << i;
> hmask = hsize - 1;
>
> --
> 1.8.4.rc3.2.g2c2b664
>
>
Philip
next prev parent reply other threads:[~2013-08-17 7:04 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-15 19:37 [PATCH] create_delta_index: simplify condition always evaluating to true Stefan Beller
2013-08-15 21:21 ` Eric Sunshine
2013-08-15 21:34 ` Stefan Beller
2013-08-15 21:46 ` Eric Sunshine
2013-08-16 2:38 ` Nicolas Pitre
2013-08-16 16:47 ` Philip Oakley
2013-08-16 21:22 ` Stefan Beller
2013-08-17 6:58 ` Philip Oakley [this message]
2013-08-15 22:14 ` Philip Oakley
2013-08-15 21:43 ` Junio C Hamano
2013-08-15 22:04 ` Stefan Beller
2013-08-16 2:34 ` Nicolas Pitre
2013-08-16 11:43 ` brian m. carlson
2013-08-16 12:40 ` Eric Sunshine
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E5FB18126E7F42D79E085F3924AC9E07@PhilipOakley \
--to=philipoakley@iee.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=nico@fluxnic.net \
--cc=stefanbeller@googlemail.com \
--cc=sunshine@sunshineco.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.