Careful tweaking to exprOkForSpeculation

This patch does several things:

* Make exprOkForSpeculation ignore evaluatedness of variables
  See the Note [exprOkForSpeculation and evaluated variables]
  This means that the binder-swap transformation no longer
  invaliates the let/app invariant.

* Make exprOkForSpeculation return False for
     DataToTagOp and SeqOp.
  See Note [exprOkForSpeculation and SeqOp/DataToTagOp]

* Remove the 'can_fail' property from dataToTag#; it was
  always a hack (described in the old Note [dataToTag#] in
  primops.txt.pp), and now its not necessary because of the
  fixes above.

* Make SetLevels use exprIsHNF, /not/ exprOkForSpeculation,
  when floating single-alternative cases.  See SetLevels
  Note [Floating single-alternative cases]

* Fix a buglet in FloatIn; probably never bites in practice
  See Note [Dead bindings]

Collectively, these changes finally fix Trac #15696.

2 files changed, 32 insertions, 56 deletions

diff --git a/compiler/prelude/PrelRules.hs b/compiler/prelude/PrelRules.hs
index ce269e36f6..cd04074545 100644
--- a/compiler/prelude/PrelRules.hs
+++ b/compiler/prelude/PrelRules.hs
@@ -1042,7 +1042,36 @@ dataToTagRule = a `mplus` b
       ASSERT( not (isNewTyCon (dataConTyCon dc)) ) return ()
       return $ mkIntVal dflags (toInteger (dataConTagZ dc))
 
-{-
+{- Note [dataToTag# magic]
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+The primop dataToTag# is unusual because it evaluates its argument.
+Only `SeqOp` shares that property.  (Other primops do not do anything
+as fancy as argument evaluation.)  The special handling for dataToTag#
+is:
+
+* CoreUtils.exprOkForSpeculation has a special case for DataToTagOp,
+  (actually in app_ok).  Most primops with lifted arguments do not
+  evaluate those arguments, but DataToTagOp and SeqOp are two
+  exceptions.  We say that they are /never/ ok-for-speculation,
+  regardless of the evaluated-ness of their argument.
+  See CoreUtils Note [exprOkForSpeculation and SeqOp/DataToTagOp]
+
+* There is a special case for DataToTagOp in StgCmmExpr.cgExpr,
+  that evaluates its argument and then extracts the tag from
+  the returned value.
+
+* An application like (dataToTag# (Just x)) is optimised by
+  dataToTagRule in PrelRules.
+
+* A case expression like
+     case (dataToTag# e) of <alts>
+  gets transformed t
+     case e of <transformed alts>
+  by PrelRules.caseRules; see Note [caseRules for dataToTag]
+
+See Trac #15696 for a long saga.
+
+
 ************************************************************************
 *                                                                      *
 \subsection{Rules for seq# and spark#}
@@ -1097,7 +1126,7 @@ Implementing seq#.  The compiler has magic for SeqOp in
 - StgCmmExpr.cgExpr, and cgCase: special case for seq#
 
 - CoreUtils.exprOkForSpeculation;
-  see Note [seq# and expr_ok] in CoreUtils
+  see Note [exprOkForSpeculation and SeqOp/DataToTagOp] in CoreUtils
 
 - Simplify.addEvals records evaluated-ness for the result; see
   Note [Adding evaluatedness info to pattern-bound variables]
diff --git a/compiler/prelude/primops.txt.pp b/compiler/prelude/primops.txt.pp
index bf69776166..eb635fb215 100644
--- a/compiler/prelude/primops.txt.pp
+++ b/compiler/prelude/primops.txt.pp
@@ -3150,65 +3150,12 @@ section "Tag to enum stuff"
 primop  DataToTagOp "dataToTag#" GenPrimOp
    a -> Int#  -- Zero-indexed; the first constructor has tag zero
    with
-   can_fail   = True -- See Note [dataToTag#]
    strictness = { \ _arity -> mkClosedStrictSig [evalDmd] topRes }
+   -- See Note [dataToTag# magic] in PrelRules
 
 primop  TagToEnumOp "tagToEnum#" GenPrimOp
    Int# -> a
 
--- Note [dataToTag#]
--- ~~~~~~~~~~~~~~~~~
--- The primop dataToTag# is unusual because it evaluates its argument.
--- Only `SeqOp` shares that property.  (Other primops do not do anything
--- as fancy as argument evaluation.)  The special handling for dataToTag#
--- is:
---
--- * CoreUtils.exprOkForSpeculation has a special case for DataToTagOp,
---   (actually in app_ok).  Most primops with lifted arguments do not
---   evaluate those arguments, but DataToTagOp and SeqOp are two
---   exceptions.  We say that they are /never/ ok-for-speculation,
---   regardless of the evaluated-ness of their argument.
---   See CoreUtils Note [PrimOps that evaluate their arguments]
---
--- * There is a special case for DataToTagOp in StgCmmExpr.cgExpr,
---   that evaluates its argument and then extracts the tag from
---   the returned value.
---
--- * An application like (dataToTag# (Just x)) is optimised by
---   dataToTagRule in PrelRules.
---
--- * A case expression like
---      case (dataToTag# e) of <alts>
---   gets transformed t
---      case e of <transformed alts>
---   by PrelRules.caseRules; see Note [caseRules for dataToTag]
---
--- See Trac #15696 for a long saga.
---
--- Note [dataToTag# hack]
--- ~~~~~~~~~~~~~~~~~~~~~~
--- (This a temporary hack: see Trac #15696 commment:60.)
---
--- dataToTag# evaluates its argument, so we don't want to float it out.
--- Consider:
---
---     \z. case x of y -> let v = dataToTag# y in ...
---
--- To improve floating, the FloatOut pass (deliberately) does a
--- binder-swap on the case, to give
---
---     \z. case x of y -> let v = dataToTag# x in ...
---
--- Now FloatOut might float that v-binding outside the \z
---
---     let v = dataToTag# x in \z. case x of y -> ...
---
--- But that is bad because that might mean x gets evaluated much too early!
---
--- Solution: make dataToTag# into a can_fail primop.  That will stop it floating
--- (see Note [PrimOp can_fail and has_side_effects] in PrimOp).  It's a bit of
--- a hack but never mind.
-
 ------------------------------------------------------------------------
 section "Bytecode operations"
         {Support for manipulating bytecode objects used by the interpreter and