Opened 3 years ago

Closed 2 years ago

#5327 closed bug (fixed)

INLINABLE pragma and newtypes prevents inlining

Reported by: reinerp Owned by: igloo
Priority: normal Milestone: 7.4.1
Component: Compiler Version: 7.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Difficulty:
Test Case: T5327 Blocked By:
Blocking: Related Tickets:

Description

Compile the following code with 'ghc -O2 -ddump-simpl':

module A where

newtype Size = Size Int

{-# INLINABLE val2 #-}
val2 = Size 0

f n = case val2 of Size s -> s + s > n

With ghc-7.1.20110629, we get the following Core:

A.f1 =
  case A.val2 `cast` (A.NTCo:Size :: A.Size ~ GHC.Types.Int)
  of _ { GHC.Types.I# x_aoz ->
  GHC.Types.I# (GHC.Prim.+# x_aoz x_aoz)
  }

A.f = \ (n_ab1 :: GHC.Types.Int) -> GHC.Classes.gtInt A.f1 n_ab1

and we get something similar with ghc-7.0.3. In particular, for both versions of ghc, the addition s+s should be simplified to 0, but isn't.

Any of the following changes will let s+s simplify to 0:

  • change newtype Size = ... into data Size = ...
  • remove the INLINABLE pragma
  • with ghc-7.0.3, turning the INLINABLE pragma into INLINE. However, with ghc-7.1.20110629, the INLINE pragma doesn't fix the problem.

Change History (4)

comment:1 Changed 3 years ago by igloo

  • Milestone set to 7.4.1

Thanks for the report.

comment:2 Changed 3 years ago by simonpj@…

commit 29edeadbc7d974232f67cdc4fd05283b35721ae0

Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Mon Jul 25 09:33:09 2011 +0100

    Re-engineer exprIsConApp_maybe (fixes Trac #5327)
    
    The problem with #5327 was like this:
        let x = I# 0 `cast` co1
        in ...(case x `cast` co2 of I# y -> blah)...
    
    The two casts cancelled out, but exprIsConApp_maybe couldn't see
    that.  This patch makes it simpler, faster, and more effective.
    
    (Incidentally, usually 'x' would be inlined, in #5327 it wasn't
    because of an INLINEABLE pragma and the lone-variable thing.
    Instead of fiddling with alrady-delicate code, I just made
    exprIsConApp_maybe better.)

 compiler/coreSyn/CoreUnfold.lhs |  153 +++++++++++++++++++--------------------
 1 files changed, 76 insertions(+), 77 deletions(-)

comment:3 Changed 3 years ago by simonpj

  • Owner set to igloo

Thanks for identifying this bug so accurately. I really appreciate that. The fix is nice too.

Ian: could you add a test? I think if you change (Size 0) to (Size 17) in the program and then grep for 34# (twice 17) in the -ddump-simpl output, that'd be a good way to check that the constant-fold had happened.

Could merge, but not a show stopper.

Simon

comment:4 Changed 2 years ago by igloo

  • Resolution set to fixed
  • Status changed from new to closed
  • Test Case set to T5327

Test added.

Note: See TracTickets for help on using tickets.