Opened 8 years ago

Last modified 6 years ago

#932 new task

Improve inlining

Reported by: simonpj Owned by: simonpj
Priority: normal Milestone:
Component: Compiler Version: 6.4.2
Keywords: Cc: samb simonpj
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Difficulty: Unknown
Test Case: N/A Blocked By:
Blocking: Related Tickets:

Description

Currently the contruct

  case (x# ># 0#) of ...

attracts no argument discount for x#, which is silly.

The comment in CoreUnfold? says:

	  PrimOpId op  -> primOpSize op (valArgCount args)
			  -- foldr addSize (primOpSize op) (map arg_discount args)
			  -- At one time I tried giving an arg-discount if a primop 
			  -- is applied to one of the function's arguments, but it's
			  -- not good.  At the moment, any unlifted-type arg gets a
			  -- 'True' for 'yes I'm evald', so we collect the discount even
			  -- if we know nothing about it.  And just having it in a primop
			  -- doesn't help at all if we don't know something more.

But the right thing to do seems to be to fix interestingArg in SimplUtils? so that it only thinks a primitive-typed thing is interesting if it knows its value (or some structure).

Here's the program that triggered this thought:

import GHC.Word
import GHC.Base
import GHC.Prim

a `shiftRLT` b | b >=# 32# = int2Word# 0#
                | otherwise = a `uncheckedShiftRL#` b

(W32# x#) `shift` (I# i#) =
{- we do an actual case analysis on i# to try to give us a discount -}
   case i# of
    {- For some bizzare reason removing the `shiftRLT` 0# makes the
       inlining fail again -}
    0# -> W32# (x# `shiftRLT` 0#)
    _ -> 
	 if i# >=# 0# then W32# (narrow32Word# (x# `shiftL#` i#))
         else W32# (x# `shiftRLT` negateInt# i#)

x `shiftR` y = x `shift` (-y)

shift7 x = x `shiftR` 7

roconnor@… initiated the thread

Attachments (1)

inliner3.dpatch (223.0 KB) - added by samb 7 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 7 years ago by igloo

  • Milestone set to 6.6.1
  • Test Case set to N/A

comment:2 Changed 7 years ago by samb

  • Cc samb added

Unless I am quite mistaken, this will not help unless the function actually has an argument discount for that argument? I did not observe this for *any* of the shift workers for the types based on Word# or Int#. (Which is the point of the patches I've made so far...)

So I guess you'd need to do *both* things?

Of course, I could easily be quite mistaken ;-). There doesn't appear to be any commentary on the Trac wiki about inlining at all.

Changed 7 years ago by samb

comment:3 Changed 7 years ago by samb

Hmm, that did not work right! Trac won't let people download the attachment?

Anyway, with that last idea my patches no longer make anything in nofib allocate more -- however they also don't make any of them allocate *less* :-(. They do make a couple of 'em slower, though.

And then there are things like this:

module Test2 where

import GHC.Exts

foo = word2Int#

bar = foo

Which produces this output when built with ghc/compiler/ghc-inplace -ddump-inlinings -no-recomp -O2 -c Test2.hs:

SimplBind [main:Test2.foo{v r7V} [lid]]
SimplBind [main:Test2.bar{v r7X} [lid]]
Considering inlining
    main:Test2.foo{v r7V} [lid] active: False
				arg infos []
				interesting continuation False
				is value: True
				is cheap: True
				guidance IF_ARGS 0 [] 1 0
				ANSWER = NO
...

Which gives word2Int# a "size" of 1, even though it doesn't DO anything.

comment:4 Changed 7 years ago by samb

  • Cc samb simonpj added; samb removed

comment:5 Changed 7 years ago by simonmar

  • Milestone changed from 6.6.1 to 6.8
  • Owner set to simonpj

comment:6 Changed 7 years ago by guest

comment:7 Changed 6 years ago by simonpj

  • Milestone changed from 6.8 branch to _|_

There's probably something that can usefully be improved here, but let's wait until someone yells a bit more about it, with some program they care about. Sam?

Simon

comment:8 Changed 6 years ago by simonmar

  • Architecture changed from Unknown to Unknown/Multiple

comment:9 Changed 6 years ago by simonmar

  • Operating System changed from Unknown to Unknown/Multiple
Note: See TracTickets for help on using tickets.