LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 32495 - Merge r292188 and r292246 into 4.0.1 (fixes bug 32494, and possibly others)
Summary: Merge r292188 and r292246 into 4.0.1 (fixes bug 32494, and possibly others)
Status: RESOLVED FIXED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: trunk
Hardware: PC All
: P normal
Assignee: Tom Stellard
URL: https://reviews.llvm.org/rL292188
Keywords:
Depends on:
Blocks: release-4.0.1 32494
  Show dependency tree
 
Reported: 2017-04-02 09:56 PDT by Dimitry Andric
Modified: 2017-05-26 07:30 PDT (History)
5 users (show)

See Also:
Fixed By Commit(s):


Attachments
log1p Fix (747 bytes, patch)
2017-05-25 09:00 PDT, Tom Stellard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitry Andric 2017-04-02 09:56:56 PDT
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.
Comment 1 Tom Stellard 2017-04-04 12:10:42 PDT
Is this OK to merge?
Comment 2 Tom Stellard 2017-05-19 08:55:20 PDT
Davide or Eli, are these patches OK for the 4.0.1 branch?
Comment 3 Davide Italiano 2017-05-19 11:57:38 PDT
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).
Comment 4 Tom Stellard 2017-05-19 13:27:10 PDT
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.
Comment 5 Dimitry Andric 2017-05-19 13:48:02 PDT
(In reply to Tom Stellard from comment #4)
> 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<FPMathOperator>(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.'
Comment 6 Tom Stellard 2017-05-24 03:25:57 PDT
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?
Comment 7 Tom Stellard 2017-05-25 09:00:53 PDT
Created attachment 18510 [details]
log1p Fix

Here is the patch with only the log1p fix.  Can you try this?
Comment 8 Dimitry Andric 2017-05-26 06:31:17 PDT
(In reply to Tom Stellard from comment #7)
> 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.
Comment 9 Tom Stellard 2017-05-26 07:30:05 PDT
Merged: r303992