Roundup Robot added the comment:
New changeset caebb4f231da by Victor Stinner in branch 'default':
Issue #20162: Fix an alignment issue in the siphash24() hash function which
http://hg.python.org/cpython/rev/caebb4f231da
--
___
Python tracker
STINNER Victor added the comment:
I applied siphash_ppc64.patch. Thanks Yury V. Zaytsev for your report and your
help to investigate this tricky bug.
--
resolution: - fixed
status: open - closed
___
Python tracker rep...@bugs.python.org
Serhiy Storchaka added the comment:
Well, then it LGTM.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20162
___
___
Python-bugs-list mailing
STINNER Victor added the comment:
@Christian: Are you ok with siphash_ppc64.patch? I'm going to push the fix.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20162
___
New submission from Yury V. Zaytsev:
Hi,
When I try the following:
./python -m test -v test_hash
on a self-compiled Python 3.4.0b2 on RHEL 6.5 / ppc64, it fails. Please let me
know which additional information I can supply to diagnose the problem.
The complete traceback below:
==
Changes by Antoine Pitrou pit...@free.fr:
--
components: +Interpreter Core
nosy: +David.Edelsohn, christian.heimes, dmalcolm
priority: normal - high
type: crash - behavior
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20162
Yury V. Zaytsev added the comment:
As requested by Victor Stinner:
./python -c 'import sys; print(sys.hash_info)'
sys.hash_info(width=64, modulus=2305843009213693951, inf=314159, nan=0,
imag=103, algorithm='siphash24', hash_bits=64, seed_bits=128, cutoff=0)
--
components:
Yury V. Zaytsev added the comment:
Sorry for accidentally rolling back your changes to the bug, Antoine!
--
components: +Interpreter Core -Tests
type: crash - behavior
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20162
Changes by Yury V. Zaytsev y...@shurup.com:
--
components: +Tests
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20162
___
___
Python-bugs-list
Roundup Robot added the comment:
New changeset 81f8b4744f1a by Victor Stinner in branch 'default':
Issue #20162: test_hash_distribution() uses subTest() to mention the prefix in
http://hg.python.org/cpython/rev/81f8b4744f1a
--
nosy: +python-dev
___
Yury V. Zaytsev added the comment:
==
FAIL: test_hash_distribution (test.test_hash.HashDistributionTestCase)
(prefix='abc')
--
Traceback (most recent call
Yury V. Zaytsev added the comment:
To check whether the problem is in the _le64toh() macro as suggested by Victor
Stinner, I've tried the attached patch and the problem is gone.
As it turns out, there actually seem to be two problems:
First, HAVE_ENDIAN_H is properly defined, because the
Yury V. Zaytsev added the comment:
I was also asked to mention this:
https://github.com/majek/csiphash/blob/master/csiphash.c
as an alternative implementation of siphash and platform checks.
--
___
Python tracker rep...@bugs.python.org
Yury V. Zaytsev added the comment:
After lots of fiddling, I can tell you what's wrong with the macro: apparently
it's a compiler bug, visible at -O2 and disappearing at -O1.
Assembly output is attached, unfortunately, I'm no ppc64 expert, so I can't
immediately tell what exactly went wrong.
Dave Malcolm added the comment:
On Tue, 2014-01-07 at 16:30 +, Yury V. Zaytsev wrote:
Yury V. Zaytsev added the comment:
After lots of fiddling, I can tell you what's wrong with the macro:
apparently it's a compiler bug, visible at -O2 and disappearing at -O1.
Can you reduce the
Yury V. Zaytsev added the comment:
Hi David,
It's gcc from RHEL 6.5 gcc-4.4.7-4.el6.ppc64, the flags are -DNDEBUG -g
-fwrapv -O2 -Wall -Wstrict-prototypes vs. -DNDEBUG -g -fwrapv -O1 -Wall
-Wstrict-prototypes.
The problem is, I can't isolate it. We honestly tried it with Victor, but gcc
Yury V. Zaytsev added the comment:
Look for _le64toh ;-)
--
Added file: http://bugs.python.org/file33347/pyhash.preproc.c
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20162
___
Yury V. Zaytsev added the comment:
Digging more into it, I guess I know why we couldn't come up with a minimal
reproducer for this problem. If I compile with -O2 instead of -O1, I get the
following warning from gcc:
Python/pyhash.c:413: warning: dereferencing pointer 'pt.32' does break
STINNER Victor added the comment:
Python/pyhash.c:413: warning: dereferencing pointer 'pt.32' does break
strict-aliasing rules
Attached patch siphash_ppc64.patch should fix this aliasing issue.
--
nosy: +haypo
Added file: http://bugs.python.org/file33349/siphash_ppc64.patch
Yury V. Zaytsev added the comment:
Related issue where memcpy() was discussed: http://bugs.python.org/issue19183 .
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20162
___
STINNER Victor added the comment:
Attached patch siphash_ppc64.patch should fix this aliasing issue.
It looks like memcpy was discussed in the initial issue #19183 for
performances. IMO the function must first produce the good result, and
then be fast :-)
--
Christian Heimes added the comment:
Oh heck ... I didn't run into this issue when I was testing siphash on all
platforms. Could it be a compiler bug? I'd rather not change the code and
deviate from the reference implementation. It's a performance critical part...
--
Serhiy Storchaka added the comment:
On platforms where unaligned access is not an issue, gcc produces for memcpy()
an optimal binary code equivalent to simple assignment of 32-bit value. Only
MSVC produces slow code, but Windows works only on platforms where unaligned
access is not an issue.
Yury V. Zaytsev added the comment:
Hi Christian,
Dave says it's not a compiler bug; the code is slightly violating the C
standard, and the compiler optimizes based on a strict reading of the rules.
If I compile with -O2 and higher (while -O3 is the default for Python, as far
as I can
STINNER Victor added the comment:
Only MSVC produces slow code, but Windows works only on platforms where
unaligned access is not an issue.
My patch uses Py_MEMCPY() which uses a loop to copy bytes on Visual
Studio for length smaller than 16 bytes (SipHash copies 4 bytes).
--
25 matches
Mail list logo
