justinethier / cyclone

:cyclone: A brand-new compiler that allows practical application development using R7RS Scheme. We provide modern features and a stable system capable of generating fast native binaries.
http://justinethier.github.io/cyclone/
MIT License
823 stars 42 forks source link

chudnovsky benchmark fails on latest version of arch #276

Closed justinethier closed 6 years ago

justinethier commented 6 years ago

With a fresh Arch Linux installation, the benchmark fails with the following error:

ERROR: returned incorrect result: (314159265358979323846264338327950288419716939937510 31415926535897932384626433832795028841971693993751058209749445923078163407289818597027832468630273813 3141592653589793238462643383279502884197169399375105820974944592307816406286208998628395732195247134736419959446064710528192818179091155023804879421112 314159265358979323846264338327950288419716939937510582097494459230781634072898185970278324686302738135983700707997737044432966460985460181099892476161840842399365953171825584444517342056435233915714212 31415926535897932384626433832795028841971693993751058209749445923078163407289818597027832468630273813598370070799773704382256145334614002360104552210794197262186972297081423708982285724384457124016319663074787614260024132144362336139424211017345601955 3141592653589793238462643383279502884197169399375105820974944592307816406286208998628395732194995286457588793121182837544599905324388739269657977338055090584894929756627772034623450752582271006263179108397471319055431575867585418409607230950739029640661339486238384581935276833507854949829075469988257 314159265358979323846264338327950288419716939937510582097494459230781640628620899862839573219524713473641995944606471052819281817909115502380487942111243480980649515550381564749480019904518586992920379344536668291909883143316365449746656709619784025661938904254023599667706867178322149824408976889019276866011248697019902050718836026381884376458646607 31415926535897932384626433832795028841971693993751058209749445923078164062862089986283957365040968600914675243277430860201044089805964056430913105676269426961079999767300266094397351515516927734877067410949170820083965377160845652969476379123128561775397507660464855100562365735403059347501548887610246681924546614034999366289945454779013445558016471932791268496126758419749090882904659792406715665067 3141592653589793238462643383279502884197169399375105820974944592307816340728981859702783246863027381359837007079977370444329664609854601810998924761618408423993659531718255844445173420564352339157142125260521728261787966711833271751851891279761610003232474297171807651459525093743699358302133686938269808355105884933726949691305527583757945497318822040770716277148596036101804931099066307231451204722861246512488537780044626541202419930796169154998678 314159265358979323846264338327950288419716939937510582097494459230781634072898185970278324686302738135983700707997737043822561453346140023601045522107941972621869722970814237089822857243844571240163196630747876142600241321488991769887068587062158272647299644302153836562388100241493565352461328270132500113602629576354848345817473018546350960607954770934097673929772094266302969225288542945627831149756308162664404372290503603367780310906792522542818612563851547976103555831598645844194634375356098780)

justinethier commented 6 years ago

[justin@justin-arch-vm r7rs-benchmarks]$ pacman -Q libtommath libtommath 1.0.1-1

justinethier commented 6 years ago

Both a working and failing Arch seem to have the same version of libtommath.

Working Arch is using gcc (GCC) 7.2.1 20171224 Failing one is using gcc (GCC) 8.2.1 20180831

Perhaps there are other differences as well...

justinethier commented 6 years ago

(ch-split 0 9) returns a different result on the newer arch.

justinethier commented 6 years ago

Both REPL's give this result:

cyclone> (* 1155 3380195)
-390842071

Seems a result that should be a bignum is returned as a fixnum. The compiled code in the newer version of arch also prints this result instead of a bignum one.

justinethier commented 6 years ago

Here is a simple test program that demonstrates the problem:

(import (scheme base) (scheme write))
(define n 1155)
(write (* n 3380195))
justinethier commented 6 years ago

Looks like an issue in Cyc_checked_mul. Maybe we are hitting undefined behavior and need to check for overflow in a different manner.

justinethier commented 6 years ago

Also, this works fine in debug mode -g but not with -O2...

With -O2 the result of the multiplication is interpreted as a fixnum when it really should be a bignum.

justinethier commented 6 years ago

Concerned about the performance impact of the above change; this may not be good enough.

justinethier commented 6 years ago

Verified the benchmark now runs successfully on the latest Arch release.