Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Determinism] Add hash based testing #224

Merged
merged 11 commits into from Oct 11, 2018

Conversation

Projects
None yet
2 participants
@shibatch
Copy link
Owner

shibatch commented Aug 31, 2018

This patch adds tester3 which does hash-based comparison of outputs from each functions.
MD5 algorithm is used for the hash functions, since public domain implementation is available.
Hash functions are calculated beforehand for function implementations with FMA and non-FMA, and they are stored in src/libm-tester/hash_[cf]inz.txt.
During a test, hash functions are calculated in the same way, and checked if the resulting hash values matche the values in those files.
VSX testing at travis is disabled since emulation is very slow.

shibatch added some commits Aug 31, 2018

@shibatch shibatch requested a review from fpetrogalli-arm Aug 31, 2018

# Build tester3
string(TOLOWER ${SIMD} SCSIMD)
set(T "tester3${SCSIMD}")
add_executable(${T} tester3.c tester3main.c testerutil.c md5.c md5.h)

This comment has been minimized.

@fpetrogalli-arm

fpetrogalli-arm Aug 31, 2018

Collaborator

Could you please link the MD5 library, instead of compiling the code in? I rather not maintain untested MD5 sources in SLEEF codebase. Have you looked into detecting the availability of the MD5 library via cmake? I guess it's C interface is not OS and system independent, but I'd rather maintain a conditional header include in the sourced of tester3 other than keeping the MD5 sources.

This comment has been minimized.

@shibatch

shibatch Aug 31, 2018

Author Owner

Usually it is not welcomed to make a link to the original server since it will increase the load of the server. I can make a new GitHub repository for these two files and make a link to there, but is that what you want?

This comment has been minimized.

@fpetrogalli-arm

fpetrogalli-arm Aug 31, 2018

Collaborator

No, sorry, I didn't mean that, I'll try to be clear. I rather have not any code that implements the MD5 algorithm, and rely on an external library.

For example, these blog shows hot to invoke the MD5 routines from C code, relying on the availability of an external library:

https://www.quora.com/What-is-the-best-way-to-create-an-MD5-hash-in-C++

Notice also that it says "that MD5 hashes can be collided easily and MD5 is deprecated", and it suggest to use other hash algorithms like SHA-256.

For SHA-256, you could base your code on this example, which uses the openssl library:

https://stackoverflow.com/questions/2262386/generate-sha256-with-openssl-and-c

This comment has been minimized.

@shibatch

shibatch Aug 31, 2018

Author Owner

This is not a cryptographic application, so hash collision is not a problem.
md5.c and md5.h are kind of external. They are used from tester3.c in a similar way as explained.
Do you mean that I should put those two files into different directory?

@fpetrogalli-arm

This comment has been minimized.

Copy link
Collaborator

fpetrogalli-arm commented Aug 31, 2018

shibatch added some commits Aug 31, 2018

@shibatch

This comment has been minimized.

Copy link
Owner Author

shibatch commented Sep 1, 2018

Okay, so MD5 functions are now external.
The exactly same functions are included in libssl.

if (NOT CMAKE_CROSSCOMPILING AND NOT SLEEF_FORCE_FIND_PACKAGE_SSL)
find_package(OpenSSL)
else()
# find_package cannot find OpenSSL when cross-compiling

This comment has been minimized.

@fpetrogalli-arm

fpetrogalli-arm Sep 4, 2018

Collaborator

Why not use directly libssl and libcrypto also when not cross compiling? I suggested OpenSSL just as an example, I didn't want to tie you to a particular implementation.

If possible, simplify this check by removing the OpenSSL branch.

This comment has been minimized.

@shibatch

shibatch Sep 5, 2018

Author Owner

find_library cannot find OpenSSL with MSVC. So, I need both.

# find_package cannot find OpenSSL when cross-compiling
find_library(LIBSSL ssl)
find_library(LIBCRYPTO crypto)
if (LIBSSL AND LIBCRYPTO)

This comment has been minimized.

@fpetrogalli-arm

fpetrogalli-arm Sep 4, 2018

Collaborator

Why do you need to set OpenSSL libraries as libssl libraries?

This comment has been minimized.

@shibatch

shibatch Sep 5, 2018

Author Owner

libssl and libcrypto are the components of OpenSSL library.

find_library(LIBCRYPTO crypto)
if (LIBSSL AND LIBCRYPTO)
set(OPENSSL_FOUND TRUE)
set(OPENSSL_LIBRARIES ${LIBSSL} ${LIBCRYPTO})

This comment has been minimized.

@fpetrogalli-arm

fpetrogalli-arm Sep 4, 2018

Collaborator

This is risky. The OPENSSL_* variables are set by "find_library" afai, and manually setting them might break other components if SLEEF is added as a cmake subproject. Please avoid doing so. If you want to be able to switch between openSSL and libssl, I recommend setting variables "SLEEF_MD5_FOUND" and "SLEEF_MD5_LIBRARIES" and "SLEEF_MD5_INCLUDES" according to the results of find_package on libopenssl and libssl, and then use those new variables in the rest of the CMAKE scripts.

set -ev
export QEMU_CPU=POWER8
cd /build/build-cross
make -j 2 all

This comment has been minimized.

@fpetrogalli-arm

fpetrogalli-arm Sep 4, 2018

Collaborator

It was nice we could claim this on PPC. I understand you are removing it because it is very slow. Why is that? Would it be possible to move it to Jenkins?

This comment has been minimized.

@shibatch

shibatch Sep 5, 2018

Author Owner

VSX testing is already made on Jenkins.
I don't know why VSX vector operations are slow on QEMU, but it is slower than AArch64 NEON emulation.

shibatch added some commits Sep 10, 2018

@shibatch shibatch merged commit 9b4ae0c into master Oct 11, 2018

6 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/branch This commit looks good
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@shibatch shibatch deleted the Add_hash_based_testing2 branch Oct 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.