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

Merge r292188 and r292246 into 4.0.1 (fixes bug 32494, and possibly others) #31842

Closed
DimitryAndric opened this issue Apr 2, 2017 · 10 comments
Closed
Assignees
Labels
bugzilla Issues migrated from bugzilla

Comments

@DimitryAndric
Copy link
Collaborator

Bugzilla Link 32495
Resolution FIXED
Resolved on May 26, 2017 07:30
Version trunk
OS All
Blocks #31409 llvm/llvm-bugzilla-archive#32494
CC @ahmedbougacha,@efriedma-quic,@tstellar

Extended Description

Please merge the following into 4.0.1:

https://reviews.llvm.org/rL292188
https://reviews.llvm.org/rL292246

The former is a fix for bug 32494 (and possibly others), the latter a follow-up fix to eliminate MSVC warnings.

@DimitryAndric
Copy link
Collaborator Author

assigned to @tstellar

@tstellar
Copy link
Collaborator

tstellar commented Apr 4, 2017

Is this OK to merge?

@tstellar
Copy link
Collaborator

Davide or Eli, are these patches OK for the 4.0.1 branch?

@llvmbot
Copy link
Collaborator

llvmbot commented May 19, 2017

This has been in tree for a while, and it seems to be useful to the FreeBSD folks. I think it's OK to merge this to 4.0.1 in some form.
The cons is that this is a large patch that could itself introduce other regressions, so if there's a way to backport a subset of it (the one that fixes the FreeBSD crash, that would be better).

Backporting the MSVC warning fix seems a no-brainer to me (assuming the other patch gets backported).

@tstellar
Copy link
Collaborator

Dimitry, is there a specific bug this patch fixes? I agree that it would be nice to merge a subset of this patch if possible.

@DimitryAndric
Copy link
Collaborator Author

Dimitry, is there a specific bug this patch fixes? I agree that it would be
nice to merge a subset of this patch if possible.

Yes, bug 32494, as mentioned in the description. Basically, this fixes
'Assertion failed: (isa(this) && "getting fast-math flag on invalid op"), function hasUnsafeAlgebra, file /usr/ports/devel/llvm40/work/llvm-4.0.0.src/lib/IR/Instruction.cpp, line 165.'

@tstellar
Copy link
Collaborator

Even though it crashes in the log1p bultin, the fact that the code is a conftest.c generated by autoconf makes me wonder if this is a generic bug that would potentially affected all builtins, or if the log1p case is special.

Are you able to patch 4.0.0 with only the log1p fix from this patch and see if you hit any other failures?

@tstellar
Copy link
Collaborator

log1p Fix
Here is the patch with only the log1p fix. Can you try this?

@DimitryAndric
Copy link
Collaborator Author

Created attachment 18510 [details]
log1p Fix

Here is the patch with only the log1p fix. Can you try this?

Yes, that works indeed, for the specific case of log1p. Should be fine for 4.0.1.

@tstellar
Copy link
Collaborator

Merged: r303992

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

3 participants