Opened 9 years ago

Closed 7 years ago

Last modified 7 years ago

#2422 closed bug (fixed)

Unrelated instance foils optimizer

Reported by: sedillard Owned by:
Priority: low Milestone: 7.0.1
Component: Compiler Version: 6.8.3
Keywords: Cc:
Operating System: Linux Architecture: x86
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

I have a type (:.) a b that I use for vectors, so a 4d vector is a:.a:.a:.a:.(). This is just a linked list, so I also have a packed representation for Doubles, Vec4D and a class that relates the two representations.

The Storable instances for (:.) are inductive: if a and (b:.c) are Storable then so is (a:.b:.c). So long as the source is a peek and the destination is a poke, the method calls optimize neatly and no (:.) values are allocated.

Here's the weird part: If I use the Storable methods of (:.) and the pack/unpack methods to extend a Storable instance to Vec4D, then the optimization of the (:.) Storable methods fails. The Vec4D instances are not used at all, and yet they affect optimization of the (:.) instances.

This is probably related to #2416. If I move the Storable instance for (:.) into Main.hs, then all optimization fails regardless of any instance for Vec4D. If I move the Storable instance for Vec4D into a different module than the instance for (:.), then optimization works fine.

The example is in two files. I tried to merge them into one but that's when I discovered #2416.

Compiled with -O2.

Attachments (2)

Main.hs (421 bytes) - added by sedillard 9 years ago.
Lib.hs (2.6 KB) - added by sedillard 9 years ago.

Download all attachments as: .zip

Change History (8)

Changed 9 years ago by sedillard

Attachment: Main.hs added

Changed 9 years ago by sedillard

Attachment: Lib.hs added

comment:1 Changed 9 years ago by igloo

difficulty: Unknown
Milestone: 6.10 branch

Good testcase, thanks for the report!

comment:2 Changed 9 years ago by simonpj

Good example! This is indeed what is making #2416 go bad, but it's much easier to see here.

I've unraveled what's happening; just recording it here so I don't forget again. These remarks are mainly for me, so they may seem a bit cryptic.

From the instance for (Storable (a:.a:.v)) we get something like

Rec {  
  $f3 = \d. MkStorable ...(pbo d)...
  pbo = \d. let pbo1 = peekByteOff ($f3 d) in \p q r. blah
end Rec }

where peekByteOff is the method selector from the Storable class. (See Note [How instance declarations are translated] in TcInstDcls for the overall game plan.) Then the simplifier inlines $f3, thereby breaking the recursion, to give

pbo = \d. let ...d... in \p q r. blah
$f3 = \d. MkStorable ...(pbo d)...

Now, when you add the "unrelated" instance, the specialiser kicks in, and makes a specialised version of both functions:

$spbo = \p q r. blah blah blah
$sf3 = MkStorable ...$spbo...

We've specialised for a particular dictionary d, and that means we can inline the specialised methods into $spbo. Alas, that makes $spbo big, so now it does not inline into the users of $sf3. Before, it did. In effect the unspecialised version looked a lot smaller than the specialised version.

But what about the INLINE pragma, you say? Well, it's hard to make it work right. If it attaches to the whole pbo we'd get

Rec {  
  $f3 = \d. MkStorable ...(pbo d)...
  pbo = __inline_me__ (\d. let pbo1 = peekByteOff ($f3 d) in \p q r. blah)
end Rec }

But (for good reasons) we don't inline inside __inline_me__, so we won't get to break the recursive loop with $f3, which is terrible. GHC 6.10 currently pins the pragma on the body thus:

Rec {  
  $f3 = \d. MkStorable ...(pbo d)...
  pbo = \d. let pbo1 = peekByteOff ($f3 d) in 
        __inline_me__ (\p q r. blah)
end Rec }

but this does not work right either (because eta expansion loses that __inline_me__).

I think the new inlining mechanism will help a lot, but I need to get it installed first. Stay tuned.

Simon

comment:3 Changed 9 years ago by igloo

Milestone: 6.10 branch6.12 branch

comment:4 Changed 7 years ago by igloo

Milestone: 6.12 branch6.12.3

comment:5 Changed 7 years ago by igloo

Milestone: 6.12.36.14.1
Priority: normallow

comment:6 Changed 7 years ago by igloo

Resolution: fixed
Status: newclosed
Type of failure: None/Unknown

The core is now the same with and without the instance.

Note: See TracTickets for help on using tickets.