Optimistic case binder CPR is wrong in the prime example
CPR is a bit optimistic about whether to give case binders the CPR property sometimes. Here's the first point of Note [CPR in a DataAlt case alternative]:
In a case alternative, we want to give some of the binders the CPR property.
Specifically
* The case binder; inside the alternative, the case binder always has
the CPR property, meaning that a case on it will successfully cancel.
Example:
f True x = case x of y { I# x' -> if x' ==# 3
then y
else I# 8 }
f False x = I# 3
By giving 'y' the CPR property, we ensure that 'f' does too, so we get
f b x = case fw b x of { r -> I# r }
fw True x = case x of y { I# x' -> if x' ==# 3 then x' else 8 }
fw False x = 3
Of course there is the usual risk of re-boxing: we have 'x' available
boxed and unboxed, but we return the unboxed version for the wrapper to
box. If the wrapper doesn't cancel with its caller, we'll end up
re-boxing something that we did have available in boxed form.
I was sceptical whether giving f the CPR property is actually a good thing, given our recent refactoring motivated by #17932 (closed)/!2929 (closed), where we decided that we don't want to give x above the CPR property. I tried the following code on HEAD as a faithful proxy for the example:
f :: Bool -> Int -> Int
f True x
| x == 3 = 8
| otherwise = x -- NB: the condition was flipped so that we can't substitute `x` for a constant here
f False _ = 3
{-# NOINLINE f #-}
And got the following Core for the worker:
Lib.$wf [InlPrag=NOINLINE] :: Bool -> Int -> GHC.Prim.Int#
[GblId, Arity=2, Str=<SU><1P(1U)>, Unf=OtherCon []]
Lib.$wf
= \ (w_sJO :: Bool) (w1_sJP :: Int) ->
case w_sJO of {
False -> 3#;
True ->
case w1_sJP of { GHC.Types.I# x_aJj ->
case x_aJj of wild2_X2 {
__DEFAULT -> wild2_X2;
3# -> 8#
}
}
}
E.g. the arg x wasn't unboxed, but $wf still returns an Int# as if the arg was a constructed product (which it isn't). This is actually in example where we shouldn't have given the case binder the CPR property! After all, the scrutinee doesn't have it either (it's a strict arg, but won't be unboxed as per wantToUnbox).
I'd say we get rid of this optimism in CPR. I think we needed it when historically CprAnal was part of DmdAnal and we analysed the case alts before the scrutinee. Now it's the other way round and we can just trust the CPR property of the scrutinee.
But this is bound to hurt some beneficial cases, like in test case CaseBinderCPR, which aims to optimise a NoFib benchmark that it better shouldn't IMO:
module CaseBinderCPR where
-- This example, taken from nofib's transform (and heavily reduced) ensures that
-- CPR information is added to a case binder
f_list_cmp::(t1 -> t1 -> Int) -> [t1] -> [t1] -> Int;
f_list_cmp a_cmp [] []= 0
f_list_cmp a_cmp [] a_ys= -1
f_list_cmp a_cmp a_xs []= 1
f_list_cmp a_cmp (a_x:a_xs) (a_y:a_ys)=
if r_order == 0
then f_list_cmp a_cmp a_xs a_ys
else r_order
where
r_order = a_cmp a_x a_y
-- But not every case binder has the CPR property.
-- x below does not and we should not CPR nestedly for it:
g :: [Int] -> (Int, Int)
g xs = let x = xs !! 0 in x `seq` (x, x)
Losing optimism means we lose CPR'ing f_list_cmp, which I'd gladly accept.
Another big motivator is that this kind of optimism severely complicated Nested CPR, where we have to decide whether to give nested fields the CPR property, as in g above. Which most of the time we don't want to do and thus have to carry a nasty field in the Cpr data type. It's quite horrible.