Fix a constant folding rule
ClosedPublic

Authored by snowleopard on Aug 26 2018, 4:58 PM.

Details

Summary

One of the constant folding rules introduced in D2858 is:

(L y :-:   v) :-: (L x :-: w) -> return $ mkL (y-x)   `add` (w `add` v)

Or, after removing syntactic noise: (y - v) - (x - w) ==> (y - x) + (w + v).
This is incorrect, since the sign of v is changed from negative to positive.
As a consequence, the following program prints 3 when compiled with -O:

-- This is just subtraction in disguise
minus :: Int -> Int -> Int
minus x y = (8 - y) - (8 - x)
{-# NOINLINE minus #-}

main :: IO ()
main = print (2 `minus` 1)

The correct rule is: (y - v) - (x - w) ==> (y - x) + (w - v).

This commit does the fix. I haven't found any other issues with the constant
folding code, but it's difficult to be certain without some automated checking.

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.
snowleopard created this revision.Aug 26 2018, 4:58 PM
bgamari edited the summary of this revision. (Show Details)Aug 26 2018, 10:22 PM
tdammers requested changes to this revision.Aug 27 2018, 10:48 AM
tdammers added a subscriber: tdammers.

Could you maybe add a test or two? You already have an example program that would highlight a regression.

This revision now requires changes to proceed.Aug 27 2018, 10:48 AM

Could you maybe add a test or two? You already have an example program that would highlight a regression.

Sure, happy to do that, but my impression is that instead of merging this patch we might actually want to revert D2858, see the discussion in https://ghc.haskell.org/trac/ghc/ticket/15569.

Anyway, if the consensus is to proceed with this patch, I'll add a few tests.

bgamari added a subscriber: hsyl20.Aug 27 2018, 1:49 PM

Let's do this: I'm going to revert D2858 from 8.6.1 since I think the patch is at the moment just too risky to include in a release. On master we can merge this patch and begin adding more testcases (@hsyl20, perhaps you could help with this?).

@bgamari Sounds good! Here is a possible way to derisk constant folding: http://ghc.haskell.org/trac/ghc/ticket/15569#comment:8

This revision was not accepted when it landed; it landed in state Needs Revision.Aug 29 2018, 9:46 AM
Closed by commit rGHC65eec9cfd441: Fix a constant folding rule (authored by snowleopard, committed by monoidal). · Explain Why
This revision was automatically updated to reflect the committed changes.

Since this caused miscompilation and we're not reverting D2858 in master, I merged as-is; tests can be added in a separate diff.