Make constructor wrappers inline only during the final phase
AbandonedPublic

Authored by aspiwack on Nov 30 2018, 5:23 PM.

Details

Summary

For case-of-known constructor to continue triggering early,
exprIsConApp_maybe is now capable of looking through lets and cases.

See Trac #15840

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Branch
master
Lint
Lint WarningsExcuse: I didn't pay much attention to long lines because there were already long lines in these files
SeverityLocationCodeMessage
Warningcompiler/basicTypes/Id.hs:69TXT3Line Too Long
Warningcompiler/coreSyn/CoreOpt.hs:776TXT3Line Too Long
Warningcompiler/coreSyn/CoreOpt.hs:806TXT3Line Too Long
Warningcompiler/coreSyn/CoreOpt.hs:831TXT3Line Too Long
Warningcompiler/coreSyn/CoreOpt.hs:854TXT3Line Too Long
Warningcompiler/coreSyn/MkCore.hs:563TXT3Line Too Long
Warningcompiler/simplCore/Simplify.hs:2384TXT3Line Too Long
Warningcompiler/simplCore/Simplify.hs:2394TXT3Line Too Long
Warningcompiler/simplCore/Simplify.hs:2400TXT3Line Too Long
Warningcompiler/simplCore/Simplify.hs:2841TXT3Line Too Long
Warningcompiler/simplCore/Simplify.hs:2847TXT3Line Too Long
Warningcompiler/simplCore/Simplify.hs:2859TXT3Line Too Long
Warningtestsuite/tests/simplCore/should_run/T15840a.hs:6TXT3Line Too Long
Unit
No Unit Test Coverage
Build Status
Buildable 25537
Build 64902: [GHC] Linux/amd64: Continuous Integration
Build 64901: [GHC] OSX/amd64: Continuous Integration
Build 64900: [GHC] Windows/amd64: Continuous Integration
Build 64899: arc lint + arc unit
aspiwack created this revision.Nov 30 2018, 5:23 PM
monoidal updated this revision to Diff 19002.Dec 4 2018, 8:40 AM
monoidal added a subscriber: monoidal.

add a missing type signature

Nofib results:

--------------------------------------------------------------------------------
        Program           Size    Allocs   Runtime   Elapsed  TotalMem
--------------------------------------------------------------------------------
             CS           0.0%      0.0%     0.186     0.186      0.0%
            CSD           0.0%      0.0%     +0.2%     +0.1%      0.0%
             FS           0.0%      0.0%     -0.3%     -0.2%      0.0%
              S           0.0%      0.0%     -1.5%     -1.5%      0.0%
             VS           0.0%      0.0%     -0.6%     -0.8%      0.0%
            VSD           0.0%      0.0%     0.008     0.008      0.0%
            VSM           0.0%      0.0%     -0.5%     -0.5%      0.0%
           anna           0.0%      0.0%     +0.1%     +0.1%      0.0%
           ansi           0.0%      0.0%     0.000     0.000      0.0%
           atom           0.0%      0.0%     -0.2%     -0.2%      0.0%
         awards           0.0%      0.0%     0.000     0.000      0.0%
         banner           0.0%      0.0%     0.000     0.000      0.0%
     bernouilli           0.0%      0.0%     +0.7%     +0.7%      0.0%
   binary-trees           0.0%      0.0%     -0.6%     -0.6%      0.0%
          boyer           0.0%      0.0%     -0.1%     -0.1%      0.0%
         boyer2           0.0%      0.0%     0.005     0.005      0.0%
           bspt           0.0%      0.0%     0.006     0.006      0.0%
       calendar           0.0%      0.0%     0.001     0.001      0.0%
       cichelli           0.0%      0.0%     0.050     0.050      0.0%
        circsim           0.0%      0.0%     -0.7%     -0.7%      0.0%
       clausify           0.0%      0.0%     +5.6%     +5.6%      0.0%
  comp_lab_zift           0.0%      0.0%     -0.7%     -0.7%      0.0%
       compress           0.0%      0.0%     -2.9%     -2.9%      0.0%
      compress2           0.0%      0.0%     +1.8%     +1.9%      0.0%
    constraints           0.0%      0.0%     +0.1%     +0.1%      0.0%
   cryptarithm1           0.0%      0.0%     +0.7%     +0.7%      0.0%
   cryptarithm2           0.0%      0.0%     0.005     0.005      0.0%
            cse           0.0%      0.0%     0.002     0.002      0.0%
   digits-of-e1           0.0%      0.0%     +5.7%     +5.7%      0.0%
   digits-of-e2           0.0%      0.0%     +0.1%     +0.1%      0.0%
          eliza           0.0%      0.0%     0.001     0.001      0.0%
          event           0.0%      0.0%     +0.0%     -0.0%      0.0%
    exact-reals          -0.0%      0.0%     +2.1%     +2.2%      0.0%
         exp3_8           0.0%      0.0%     -0.5%     -0.5%      0.0%
         expert           0.0%      0.0%     0.000     0.000      0.0%
 fannkuch-redux           0.0%     +0.0%     +1.6%     +1.6%      0.0%
          fasta           0.0%      0.0%     -1.6%     -1.6%      0.0%
            fem          +0.0%      0.0%     0.015     0.015      0.0%
            fft          +0.0%     +2.1%     +1.6%     +1.6%      0.0%
           fft2           0.0%      0.0%     +0.5%     +0.4%      0.0%
       fibheaps          -0.0%      0.0%     -0.8%     -0.8%      0.0%
           fish           0.0%      0.0%     0.006     0.007      0.0%
          fluid           0.0%      0.0%     0.005     0.005      0.0%
         fulsom           0.0%      0.0%     -0.6%     -0.6%      0.0%
         gamteb          +0.0%      0.0%     +0.2%     +0.2%     -0.7%
            gcd           0.0%      0.0%     -0.4%     -0.4%      0.0%
    gen_regexps           0.0%      0.0%     -0.0%     -0.1%      0.0%
         genfft          -0.0%     +0.0%     -2.3%     -2.4%      0.0%
             gg           0.0%      0.0%     0.007     0.007      0.0%
           grep           0.0%      0.0%     0.001     0.001      0.0%
         hidden           0.0%      0.0%     -0.8%     -0.9%      0.0%
            hpg           0.0%      0.0%     -3.6%     -3.6%      0.0%
            ida           0.0%      0.0%    +11.2%    +11.3%      0.0%
          infer           0.0%      0.0%     0.036     0.036      0.0%
        integer           0.0%      0.0%     -1.2%     -1.2%      0.0%
      integrate           0.0%      0.0%     -0.3%     -0.2%      0.0%
   k-nucleotide           0.0%      0.0%     +0.5%     +0.5%      0.0%
          kahan           0.0%      0.0%     -0.2%     -0.2%      0.0%
        knights           0.0%      0.0%     -5.8%     -5.9%      0.0%
         lambda           0.0%      0.0%     -7.7%     -7.7%      0.0%
     last-piece           0.0%      0.0%     -4.9%     -4.9%      0.0%
           lcss           0.0%      0.0%     -3.0%     -3.0%      0.0%
           life           0.0%      0.0%     0.145     0.145      0.0%
           lift           0.0%      0.0%     0.001     0.001      0.0%
         linear           0.0%      0.0%     +1.1%     +1.0%      0.0%
      listcompr           0.0%      0.0%     0.060     0.060      0.0%
       listcopy           0.0%      0.0%     0.063     0.063      0.0%
       maillist           0.0%     +0.0%     0.041     0.041    -38.3%
         mandel           0.0%      0.0%     0.036     0.036      0.0%
        mandel2           0.0%      0.0%     0.003     0.003      0.0%
           mate           0.0%      0.0%     -3.6%     -3.6%      0.0%
        minimax           0.0%      0.0%     0.002     0.002      0.0%
        mkhprog           0.0%      0.0%     0.002     0.002      0.0%
     multiplier           0.0%      0.0%     -0.7%     -0.7%      0.0%
         n-body           0.0%      0.0%     +0.2%     +0.2%      0.0%
       nucleic2           0.0%      0.0%     0.041     0.041      0.0%
           para           0.0%      0.0%     0.175     0.175      0.0%
      paraffins           0.0%      0.0%     -0.5%     -0.5%      0.0%
         parser           0.0%      0.0%     0.018     0.018      0.0%
        parstof           0.0%      0.0%     0.004     0.004      0.0%
            pic           0.0%      0.0%     0.005     0.005      0.0%
       pidigits           0.0%      0.0%     -0.1%     -0.1%      0.0%
          power           0.0%      0.0%     -1.4%     -1.4%      0.0%
         pretty           0.0%      0.0%     0.000     0.000      0.0%
         primes           0.0%      0.0%     +7.7%     +7.7%      0.0%
      primetest           0.0%      0.0%     -0.6%     -0.5%      0.0%
         prolog           0.0%      0.0%     0.001     0.001      0.0%
         puzzle           0.0%      0.0%     0.081     0.081      0.0%
         queens           0.0%      0.0%     -3.6%     -3.6%      0.0%
        reptile           0.0%      0.0%     0.006     0.006      0.0%
reverse-complem           0.0%      0.0%     -0.4%     -0.3%      0.0%
        rewrite           0.0%      0.0%     -2.3%     -2.3%      0.0%
           rfib           0.0%      0.0%     -0.1%     -0.1%      0.0%
            rsa           0.0%      0.0%     -0.2%     -0.2%      0.0%
            scc           0.0%      0.0%     0.000     0.000      0.0%
          sched           0.0%      0.0%    -12.9%    -13.0%      0.0%
            scs          +0.0%      0.0%     -3.8%     -3.7%      0.0%
         simple           0.0%      0.0%     0.122     0.122      0.0%
          solid           0.0%      0.0%     -1.9%     -1.9%      0.0%
        sorting           0.0%      0.0%     0.001     0.001      0.0%
  spectral-norm           0.0%      0.0%     +0.3%     +0.2%      0.0%
         sphere           0.0%      0.0%     +8.5%     +8.6%      0.0%
         symalg           0.0%      0.0%     -3.9%     -3.8%      0.0%
            tak           0.0%      0.0%     -0.6%     -0.6%      0.0%
      transform           0.0%      0.0%     -0.3%     -0.4%      0.0%
       treejoin           0.0%      0.0%     0.087     0.087      0.0%
      typecheck           0.0%      0.0%    -16.1%    -16.1%      0.0%
        veritas          +0.0%      0.0%     0.001     0.001      0.0%
           wang           0.0%      0.0%     +8.7%     +8.8%      0.0%
      wave4main           0.0%      0.0%     +0.2%     +0.2%      0.0%
   wheel-sieve1           0.0%      0.0%     -0.1%     -0.1%      0.0%
   wheel-sieve2           0.0%      0.0%     +0.8%     +0.8%      0.0%
           x2n1           0.0%      0.0%     -0.2%     -0.1%      0.0%
--------------------------------------------------------------------------------
            Min          -0.0%      0.0%    -16.1%    -16.1%    -38.3%
            Max          +0.0%     +2.1%    +11.2%    +11.3%      0.0%
 Geometric Mean          -0.0%     +0.0%     -0.6%     -0.6%     -0.4%
monoidal updated this revision to Diff 19009.Dec 4 2018, 4:55 PM

fix build

Generally looking good. A few suggestions.

Also can you use -ticky to find out what is happening here

fft          +0.0%     +2.1%     +1.6%     +1.6%      0.0%

I don't think things should get worse! Ever.

compiler/basicTypes/MkId.hs
412

We can accommodate floats here perfectly well

compiler/coreSyn/CoreOpt.hs
770

Let's *not* return them reversed! That's very unusual. Either reverse them here or use OrdList and turn it into a list at the end.

compiler/prelude/PrelRules.hs
1042

Yes, dataToTag# is strict, so you can float anything just fine. Let's do that.

compiler/simplCore/Simplify.hs
2404

I don't get this. It's as if we had

case (let F in K a b) of
  ...
  K c d -> rhs

where the floats F and the K a b are all OutExprs. So we want to behave exactly lke

let F in 
case K a b of
   ...
   K c d -> rhs

That is, we can simply add F around any floats returned by that inner case expression. No need for this "if we have float binds we cannot float past them".

2855

Same here

aspiwack added inline comments.Dec 6 2018, 10:23 AM
compiler/basicTypes/MkId.hs
412

Do you think it's worth it though? Dictionaries don't have wrappers, which was the case that we were trying to optimise.

compiler/coreSyn/CoreOpt.hs
770

Will do.

compiler/prelude/PrelRules.hs
1042

Ok.

compiler/simplCore/Simplify.hs
2404

The problem are the floats in the right-hand side, because their free variables could get out of scope.

case u of
  K c d -> let x = c in rhs'

cannot transform to

let x = c in
case u of
  K c d -> rhs'

But now that you mention it, maybe I'm wrapping to many floats here. As the floats from the scrutiny are free from this particular issue, they should float up.

The crux of the issue, really, is that we are using a different notion of float than the rest of the simplifier. A more robust solution would probably be to make the simplifier capable of floating single-alternative cases. But I want to avoid making too big leaps in one step.

simonpj added inline comments.Dec 6 2018, 11:49 AM
compiler/basicTypes/MkId.hs
412

Well I don't know for sure, but there seems no reason not to do it.

compiler/simplCore/Simplify.hs
2404

Well wfloats wraps them all. So something like

[] -> return (wfloats `addFloats` floats1 `addFloats` floats2, expr')

Maybe your point is that the simplifier doesnt' have case-floats. Ah yes, that makes more sense. If the wfloats are empty, then there can't possibly be a case-float. If non-empty, there might be, so we just give up and wrap. Worst-case the simplifer just wraps those floats which means that it may take one more iteration to deal with them.

A cleverer alternative would be to decide if wfloats had any case-floats; if so, do what you do, if not just addFloats on the front of floats1 addFloats floats2.

bgamari added inline comments.Jan 20 2019, 5:54 PM
compiler/basicTypes/MkId.hs
703

Shouldn't this read "chance to fire"? I believe the text was correct before.

What is the status of this, @monoidal and @aspiwack?

@bgamari We're currently working on fixing the issues found during review.