New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 22376 - Negative zero misoptimized
Summary: Negative zero misoptimized
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: Scalar Optimizations (show other bugs)
Version: trunk
Hardware: PC Windows NT
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-28 17:01 PST by Sunil Srivastava
Modified: 2015-01-29 16:23 PST (History)
3 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sunil Srivastava 2015-01-28 17:01:32 PST
The following example fails with –O2 (passes at -O1)

extern "C"  int printf(const char *, ...);
int signbit(double d) {return *((long long *)&d)<0;}
double m0 = -0.;
int main() {
  if ( m0 == 0) {
    if  (signbit(m0) != 0)
      printf("Passed\n");
    else
      printf("Failed\n");
  }
}

It is a regression since r225660, which was for Bug 17713.
Comment 1 Sanjay Patel 2015-01-28 17:54:02 PST
Negative zero; yep, that's a bug. 

Simplified test case; this should return '1', but 'opt -gvn' will cause it to return '0':

@m0 = global double -0.000000e+00, align 8

define i32 @main() {
entry:
  %0 = load double* @m0, align 8
  %cmp = fcmp oeq double %0, 0.000000e+00
  br i1 %cmp, label %if.then, label %return

if.then:                                          ; preds = %entry
  %1 = bitcast double %0 to i64
  %.lobit = lshr i64 %1, 63
  %2 = trunc i64 %.lobit to i32
  br label %return

return:                                           ; preds = %if.then, %entry
  %retval.0 = phi i32 [ %2, %if.then ], [ 2, %entry ]
  ret i32 %retval.0
}
Comment 2 Sanjay Patel 2015-01-28 18:57:50 PST
Here's the simplest patch I could come up with. If this looks like a good fix to you, I'll submit a patch with test case for review tomorrow.

Index: lib/Transforms/Scalar/GVN.cpp
===================================================================
--- lib/Transforms/Scalar/GVN.cpp	(revision 227320)
+++ lib/Transforms/Scalar/GVN.cpp	(working copy)
@@ -2182,8 +2182,11 @@
 
       // Handle the floating point versions of equality comparisons too.
       if ((isKnownTrue && Cmp->getPredicate() == CmpInst::FCMP_OEQ) ||
-          (isKnownFalse && Cmp->getPredicate() == CmpInst::FCMP_UNE))
-        Worklist.push_back(std::make_pair(Op0, Op1));
+          (isKnownFalse && Cmp->getPredicate() == CmpInst::FCMP_UNE)) {
+        // FP -0.0 and 0.0 compare equal, so we can't propagate that.
+        if (!isa<ConstantFP>(Op1) || !cast<ConstantFP>(Op1)->isZero())
+          Worklist.push_back(std::make_pair(Op0, Op1));
+      }
 
       // If "A >= B" is known true, replace "A < B" with false everywhere.
       CmpInst::Predicate NotPred = Cmp->getInversePredicate();
Comment 3 Sanjay Patel 2015-01-29 11:42:49 PST
Patch posted for review:
http://reviews.llvm.org/D7257

Sunil, Warren - I didn't find Phabricator IDs for you. Please add yourselves if you'd like to comment there.
Comment 4 Sanjay Patel 2015-01-29 16:23:51 PST
Fix checked in:
http://llvm.org/viewvc/llvm-project?view=revision&revision=227491