Re: [PATCH lt/block-sha1 0/2 v3] block-sha1: avoid pointer conversion that violates alignment constraints

2012-07-22 Thread Junio C Hamano
Jonathan Nieder  writes:

> Junio C Hamano wrote:
>> Jonathan Nieder  writes:
>
>>> The big-endian part was just my idiocy, sorry.
>>
>> Hrm, do we want an update log message for 1/2 then?
>
> Hm, I thought all the crazy had been eliminated already.
>
> *looks again*
>
> I guess "using a single 32-bit load" makes it sound like it's using a
> big-endian load instead of a load followed by twiddling in registers.
>
> Simplest fix would be to drop the phrase "by using a single 32-bit
> load", leaving "... and gcc takes full advantage, resulting in a whole
> bunch of unaligned access traps."

I was just being stupid, misread the original code and did not
realize that the generic code quoted in your log message:

#define get_be32(p) ( \
... byte-at-a-time implementation )

is not limited to big endian boxes [*1*].  So I think the message is
fine as-is.


[Footnote]

*1* In other words, the ntohl(*(uint *)(p)) is used only on selected
little endian boxes, but that does not mean the generic code was
big-endian only.  Little endian boxes that are not listed in #if
block do use the generic byte-at-a-time macro.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH lt/block-sha1 0/2 v3] block-sha1: avoid pointer conversion that violates alignment constraints

2012-07-22 Thread Jonathan Nieder
Junio C Hamano wrote:
> Jonathan Nieder  writes:

>> The big-endian part was just my idiocy, sorry.
>
> Hrm, do we want an update log message for 1/2 then?

Hm, I thought all the crazy had been eliminated already.

*looks again*

I guess "using a single 32-bit load" makes it sound like it's using a
big-endian load instead of a load followed by twiddling in registers.

Simplest fix would be to drop the phrase "by using a single 32-bit
load", leaving "... and gcc takes full advantage, resulting in a whole
bunch of unaligned access traps."  Would that work for you, or should
I resend?

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH lt/block-sha1 0/2 v3] block-sha1: avoid pointer conversion that violates alignment constraints

2012-07-22 Thread Junio C Hamano
Jonathan Nieder  writes:

> Junio C Hamano wrote:
>
>> Thanks.
>>
>> Did somebody actually compiled Git for Alpha, and even more
>> surprisingly on a big-endian variant of one?
>
> Logs from building for Alpha and running the test suite are here:
>
>  http://buildd.debian-ports.org/status/logs.php?pkg=git&arch=alpha
>
> The big-endian part was just my idiocy, sorry.

Hrm, do we want an update log message for 1/2 then?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH lt/block-sha1 0/2 v3] block-sha1: avoid pointer conversion that violates alignment constraints

2012-07-22 Thread Jonathan Nieder
Junio C Hamano wrote:

> Thanks.
>
> Did somebody actually compiled Git for Alpha, and even more
> surprisingly on a big-endian variant of one?

Logs from building for Alpha and running the test suite are here:

 http://buildd.debian-ports.org/status/logs.php?pkg=git&arch=alpha

The big-endian part was just my idiocy, sorry.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH lt/block-sha1 0/2 v3] block-sha1: avoid pointer conversion that violates alignment constraints

2012-07-22 Thread Junio C Hamano
Jonathan Nieder  writes:

> This is a series of two patches: the first avoids alignment faults
> that were making git either slow on Alpha machines or crashy,
> depending on the machine's configuration, and the second patch is a
> cosmetic nit noticed while reviewing the first.
> ...
> Thoughts?
> Jonathan

Thanks.

Did somebody actually compiled Git for Alpha, and even more
surprisingly on a big-endian variant of one?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH lt/block-sha1 0/2 v3] block-sha1: avoid pointer conversion that violates alignment constraints

2012-07-22 Thread Jonathan Nieder
Hi Junio,

This is a series of two patches: the first avoids alignment faults
that were making git either slow on Alpha machines or crashy,
depending on the machine's configuration, and the second patch is a
cosmetic nit noticed while reviewing the first.

Patches are based against

  30ae47b4 remove ARM and Mozilla SHA1 implementations, 2009-08-17

(aka the tip of the lt/block-sha1 branch) for no particular reason.
They should work fine against a more modern codebase if you prefer
that.

No changes since v2[1] except to commit messages.

I think this should be ready for application.  Thanks to Michael and
Linus for shaping the original patch into something sane.

Thoughts?
Jonathan

[1] http://thread.gmane.org/gmane.comp.version-control.git/201434/focus=201456

Jonathan Nieder (2):
  block-sha1: avoid pointer conversion that violates alignment
constraints
  block-sha1: put expanded macro parameters in parentheses

 block-sha1/sha1.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html