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

Fix #10124 by adding check for undefined behaviour #14520

Merged
merged 1 commit into from
Jan 1, 2016
Merged

Conversation

simonbyrne
Copy link
Contributor

Fixes #10124, also adds explanation of logic. Checked on ARM machine.

cc: @eschnett, @nkottary, @ViralBShah

@ViralBShah ViralBShah added the system:arm ARMv7 and AArch64 label Dec 31, 2015
@ViralBShah
Copy link
Member

Looks like this is failing the whitespace test.

@simonbyrne
Copy link
Contributor Author

Updated. Is there an easy way to add the whitespace check as a pre-commit hook?

@simonbyrne
Copy link
Contributor Author

Ah, figured it out:

cp .git/hooks/pre-commit.sample .git/hooks/pre-commit

@vtjnash
Copy link
Sponsor Member

vtjnash commented Dec 31, 2015

is this overly conservative?

+# - typemax(Ti) can't be exactly represented by Tf:

for example, typemax(Int32) can be exactly represented in Float64

@simonbyrne
Copy link
Contributor Author

Yes, these methods would not be valid in that case (note that Int32 vs Float64 are defined elsewhere).

The extra condition that I've added (fy != $(Tf(typemax(Ti)))) assumes that Tf(typemax(Ti))) involves some sort of rounding (either to 2^n or +Inf).

vtjnash added a commit that referenced this pull request Jan 1, 2016
Fix #10124 by adding check for undefined behaviour
@vtjnash vtjnash merged commit 03a5b9f into master Jan 1, 2016
@vtjnash vtjnash deleted the sb/intfloat-eqchk branch January 1, 2016 23:47
@yuyichao
Copy link
Contributor

yuyichao commented Jan 2, 2016

Hmmm, I suspect this PR has turned a similar test failure on AArch64 into an abort. Not exactly sure what happens yet trying to get a backtrace.

@yuyichao
Copy link
Contributor

yuyichao commented Jan 2, 2016

Actually the failing test passes and it is probably something else going wrong.

simonbyrne added a commit that referenced this pull request Jan 9, 2016
tkelman added a commit to tkelman/julia that referenced this pull request Jan 31, 2016
this may actually have been worked around by JuliaLang#14520, not sure
tkelman added a commit that referenced this pull request Jan 31, 2016
this may actually have been worked around by #14520, not sure
tkelman added a commit that referenced this pull request Jan 31, 2016
this may actually have been worked around by #14520, not sure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system:arm ARMv7 and AArch64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants