When is reallyUnsafePtrEquality# OK for speculation?
The notes in GHC are somewhat inconsistent on this topic.
In GHC.Core.Utils
, we see:
Note [Primops with lifted arguments]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Is this ok-for-speculation (see #13027)?
reallyUnsafePtrEquality# a b
Well, yes. The primop accepts lifted arguments and does not
evaluate them. Indeed, in general primops are, well, primitive
and do not perform evaluation.
Bottom line:
* In exprOkForSpeculation we simply ignore all lifted arguments.
* In the rare case of primops that /do/ evaluate their arguments,
(namely DataToTagOp and SeqOp) return False; see
Note [exprOkForSpeculation and evaluated variables]
In primops.txt.pp
, however, we see:
Note [reallyUnsafePtrEquality# can_fail]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
reallyUnsafePtrEquality# can't actually fail, per se, but we mark it
can_fail anyway. Until 5a9a1738023a, GHC considered primops okay for
speculation only when their arguments were known to be forced. This was
unnecessarily conservative, but it prevented reallyUnsafePtrEquality# from
floating out of places where its arguments were known to be forced.
Unfortunately, GHC could sometimes lose track of whether those arguments
were forced, leading to let/app invariant failures (see #13027 and the
discussion in #11444). Now that ok_for_speculation skips over lifted
arguments, we need to explicitly prevent reallyUnsafePtrEquality#
from floating out. Imagine if we had
\x y . case x of x'
DEFAULT ->
case y of y'
DEFAULT ->
let eq = reallyUnsafePtrEquality# x' y'
in ...
If the let floats out, we'll get
\x y . let eq = reallyUnsafePtrEquality# x y
in case x of ...
The trouble is that pointer equality between thunks is very different
from pointer equality between the values those thunks reduce to, and the latter
is typically much more precise.
Look also at this code in GHC.Core.Utils.app_ok
:
primop_arg_ok :: TyBinder -> CoreExpr -> Bool
primop_arg_ok (Named _) _ = True -- A type argument
primop_arg_ok (Anon _ ty) arg -- A term argument
| isUnliftedType (scaledThing ty) = expr_ok primop_ok arg
| otherwise = True -- See Note [Primops with lifted arguments]
Here we specifically say "if the arguments are lifted, then YES". However that's precisely what we want to avoid, as per Note [reallyUnsafePtrEquality# can_fail]
. Moreover, this code is actually wrong because reallyUnsafePtrEquality#
is levity-polymorphic, and thus isUnliftedType
panics on its uninstantiated type (a fact that was hidden by #20837 (closed)).
I'm left with a few questions:
- Would it be better to have special logic in
GHC.Core.Utils.app_ok
to account forreallyUnsafePtrEquality#
and remove thecan_fail
property inprimops.txt.pp
? That would be less roundabout, I think. - Why are we checking the uninstantiated types of the primop in
app_ok
? Shouldn't we be checking the instantiation instead, given that some primops are representation-polymorphic?
I'm happy to implement whatever solution we agree on. @simonpj?