Fix treatment of -0.0
ClosedPublic

Authored by bgamari on Jul 10 2015, 4:34 AM.

Details

Summary

Here we fix a few mis-optimizations that could occur in code with
floating point comparisons with -0.0. These issues arose from our
insistence on rewriting equalities into case analyses and the
simplifier's ignorance of floating-point semantics.

For instance, in Trac Trac #10215 (and the similar issue Trac Trac #9238) we
turned ds == 0.0 into a case analysis,

case ds of
    __DEFAULT -> ...
    0.0 -> ...

Where the second alternative matches where ds is +0.0 and *also* -0.0.
However, the simplifier doesn't realize this and will introduce a local
inlining of ds = -- +0.0 as it believes this is the only
value that matches this pattern.

Instead of teaching the simplifier about floating-point semantics
we simply prohibit case analysis on floating-point scrutinees and keep
this logic in the comparison primops, where it belongs.

We do several things here,

  • Add test cases from relevant tickets
  • Clean up a bit of documentation
  • Desugar literal matches against floats into applications of the appropriate equality primitive instead of case analysis
  • Add a CoreLint to ensure we don't pattern match on floats in Core
Test Plan

validate with included testcases

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bgamari updated this revision to Diff 3498.Jul 10 2015, 4:34 AM
bgamari retitled this revision from to Fix treatment of -0.0.
bgamari updated this object.
bgamari edited the test plan for this revision. (Show Details)
bgamari added a reviewer: simonpj.
bgamari updated the Trac tickets for this revision.
bgamari updated this revision to Diff 3499.Jul 10 2015, 4:36 AM
bgamari edited edge metadata.
  • Wordsmithing
simonpj requested changes to this revision.Jul 10 2015, 6:03 AM
simonpj edited edge metadata.

Good; see a couple of comments.

compiler/coreSyn/CoreLint.hs
658

Define a new function isFloatingTy to use here.
Put it in TcType, with isFloatTy, isDoubleTy etc.

659

Expand Note [Literal alternatives] in CoreSyn to document the fact that literals can't be floating.

This revision now requires changes to proceed.Jul 10 2015, 6:03 AM
bgamari updated this revision to Diff 3502.Jul 10 2015, 6:51 AM
bgamari edited edge metadata.
  • Add test from Trac #9238
  • Add testcase from Trac #10215
  • Match: Clean up documentation
  • HsPat: Remove redundant LANGUAGE pragma
  • deSugar: Desugar literal matches against floats to equality checks
  • CoreLint: Warn against scrutinizing floats in case
  • PrelRules: Don't turn floating-point comparisons into case analyses
  • Wordsmithing
  • Rework
bgamari updated this revision to Diff 3503.Jul 10 2015, 7:07 AM
bgamari edited edge metadata.
  • Use checkL
  • More notes
bgamari updated this revision to Diff 3504.Jul 10 2015, 7:26 AM
bgamari marked 2 inline comments as done.
  • Fix syntax error
bgamari updated this revision to Diff 3505.Jul 10 2015, 8:18 AM
  • Missing import
bgamari updated this revision to Diff 3507.Jul 10 2015, 9:27 AM
  • Bah, inverted logic
bgamari updated this revision to Diff 3508.Jul 10 2015, 10:03 AM
  • CoreLint: Right, only want to check for literal patterns

I believe this should do it.

thomie requested changes to this revision.Jul 20 2015, 12:52 PM
thomie added a reviewer: thomie.

Harbormaster says the following tests are failing: "ds032 cgrun028 T8103 Simple2"

This revision now requires changes to proceed.Jul 20 2015, 12:52 PM
bgamari updated this revision to Diff 3652.Jul 23 2015, 5:00 PM
bgamari edited edge metadata.
  • Start rework
  • Progress
  • More fixes
bgamari updated this revision to Diff 3655.Jul 24 2015, 2:29 AM
bgamari edited edge metadata.
  • Start rework
  • Progress
  • More fixes

I've pushed a new patch that implements a complete new approach. My previous approach of lowering literal matches to view patterns while tidying was quite fundamentally flawed as demonstrated by the core lint issues that it caused.

Here I've implemented what in hindsight seems like the obviously correct way of managing the issue: just change the matching logic (namely matchLiterals). A match against a floating-point literal,

case x of
  1.0 -> body

will now be desugared to,

case x :: Double of
  D# x' | 1.0# ==## x' -> body
bgamari updated this revision to Diff 3656.Jul 24 2015, 4:15 AM
  • Fix up comments
thomie resigned from this revision.Aug 29 2015, 10:12 AM
thomie removed a reviewer: thomie.
bgamari updated this revision to Diff 4379.Oct 2 2015, 4:38 AM
  • Update with Simon's patch
bgamari updated this revision to Diff 4380.Oct 2 2015, 4:57 AM
bgamari edited edge metadata.

Add missing function definitions

Here is a much cleaner approach due to SPJ. I think this is (finally) ready to be merged.

bgamari updated this object.Oct 2 2015, 7:06 AM
bgamari edited edge metadata.
This revision was automatically updated to reflect the committed changes.