Re: [PATCH lt/block-sha1 0/2 v3] block-sha1: avoid pointer conversion that violates alignment constraints
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
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
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
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
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
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