Primops: can_fail vs. has_side_effect
I thought that I should write a ticket about this while it's still fresh in my head after !2525 (closed).
Primops can be marked can_fail
and/or has_side_effects
. According to Note [PrimOp can_fail and has_side_effects]
and Note [Transformations affected by can_fail and has_side_effects]
-
can_fail
applies to primops which may throw imprecise exceptions. In terms of transformations, we may never float these out. But we may still duplicate, drop or float them in. -
has_side_effects
applies to primops which have a write effect (on memory or "the real world") or throw a precise exception (i.e. onlyraiseIO#
). (Because of #3207 (closed), we also mark read side effects this way, but that is too conservative.) Compared tocan_fail
, we may not duplicate or drop these. (Only the former property seems relevant for read effects according to #3207 (closed), but I outline a design in #3207 (comment 257470) which allows duplication and floating out read-only effects.) For this reason, it is important thatraiseIO#
is not onlycan_fail
, because we may not drop such a side-effect.
That made me thinking: There should be no need to ever set can_fail
when has_side_effects
is set, the latter implies the former. And in fact, the only way in which we use both is to define primOpOkForSideEffects
/primopOkForSpeculation
:
primOpOkForSpeculation :: PrimOp -> Bool
-- See Note [PrimOp can_fail and has_side_effects]
-- See comments with GHC.Core.Utils.exprOkForSpeculation
-- primOpOkForSpeculation => primOpOkForSideEffects
primOpOkForSpeculation op
= primOpOkForSideEffects op
&& not (primOpOutOfLine op || primOpCanFail op)
-- I think the "out of line" test is because out of line things can
-- be expensive (eg sine, cosine), and so we may not want to speculate them
primOpOkForSideEffects :: PrimOp -> Bool
primOpOkForSideEffects op
= not (primOpHasSideEffects op)
These are the only call sites of primOpHasSideEffects
and primOpCanFail
.
So why not refactor the whole can_fail
/has_side_effects
story to
data PrimOpTrait
= Droppable
-- ^ Drop the primop call.
| Dupable
-- ^ Duplicate the primop call.
| Deferrable
-- ^ Force other expressions before the primop call. Note that may entail
-- dropping the call altogether when forcing diverges.
| Speculatable
-- ^ Force the primop call even if evaluation order would prescribe
-- forcing something else first.
purePrimOpTraits, readEffectTraits, writeEffectTraits, raiseIOTraits :: [PrimOpTrait]
purePrimOpTraits = [Droppable, Dupable, Deferrable, Speculatable]
readEffectTraits = [Droppable, Deferrable, Speculatable]
writeEffectTraits = [ Deferrable] -- Although the current implementation does not defer side effects, so we are fine with raiseIO# treated as one
raiseIOTraits = []
exprSatisfiesPrimOpTraits :: [PrimOpTrait] -> CoreExpr -> Bool
This has the advantage of compositionally describing the semantics of the different kinds of effects in terms of the clear semantics of PrimOpTraits
and not having to repeat the individual traits over and over again in primops.txt.pp
. Note how easy it is to define and change readEffectTraits
compared to setting individual flags over and over again. Also now have a shot at defining readEffectTraits
in the first place. Should there be some transformation in the future that would need another trait, that is as simple as changing the few lines I just gave.
What do you think? CC @simonpj
A fix to this ticket should also address the last two points of !2525 (comment 257945).