Revert "Add -faggressive-primops"

This reverts commit 745ec959ff647c3a455767d20f6f37e9a0cc65aa.

Sigh. Seg fault

5 files changed, 22 insertions, 73 deletions

diff --git a/compiler/main/StaticFlagParser.hs b/compiler/main/StaticFlagParser.hs
index e6b0d4cb94..07eb214f74 100644
--- a/compiler/main/StaticFlagParser.hs
+++ b/compiler/main/StaticFlagParser.hs
@@ -192,7 +192,6 @@ isStaticFlag f =
     "static",
     "fhardwire-lib-paths",
     "funregisterised",
-    "faggressive-primops",
     "fcpr-off",
     "ferror-spans",
     "fPIC",
diff --git a/compiler/main/StaticFlags.hs b/compiler/main/StaticFlags.hs
index b6b8270af8..cfbd4bab58 100644
--- a/compiler/main/StaticFlags.hs
+++ b/compiler/main/StaticFlags.hs
@@ -62,7 +62,6 @@ module StaticFlags (
 	opt_SimplExcessPrecision,
 	opt_NoOptCoercion,
 	opt_MaxWorkerArgs,
-        opt_AggressivePrimOps,
 
 	-- Unfolding control
 	opt_UF_CreationThreshold,
@@ -313,11 +312,6 @@ opt_NoStateHack			= lookUp  (fsLit "-fno-state-hack")
 opt_CprOff :: Bool
 opt_CprOff			= lookUp  (fsLit "-fcpr-off")
 	-- Switch off CPR analysis in the new demand analyser
-
-opt_AggressivePrimOps :: Bool
-opt_AggressivePrimOps		= lookUp  (fsLit "-faggressive-primops")
-        -- See Note [Aggressive PrimOps] in PrimOp
-
 opt_MaxWorkerArgs :: Int
 opt_MaxWorkerArgs		= lookup_def_int "-fmax-worker-args" (10::Int)
 
diff --git a/compiler/prelude/PrimOp.lhs b/compiler/prelude/PrimOp.lhs
index 50803fb2a2..39bee1fb9d 100644
--- a/compiler/prelude/PrimOp.lhs
+++ b/compiler/prelude/PrimOp.lhs
@@ -356,19 +356,6 @@ Consequences:
   the writeMutVar will be performed in both branches, which is
   utterly wrong.
 
-  Example of a worry about float-in:
-      case (writeMutVar v i s) of s' ->
-      if b then return s'
-           else error "foo"
-  Then, since s' is used only in the then-branch, we might float
-  in to get
-      if b then case (writeMutVar v i s) of s' -> returns s'
-           else error "foo"
-  So in the 'else' case the write won't happen.  The same is
-  true if instead of writeMutVar you had some I/O performing thing.
-  Is this ok?  Yes: if you care about this you should be using 
-  throwIO, not throw.
-
 * You cannot duplicate a has_side_effect primop.  You might wonder
   how this can occur given the state token threading, but just look
   at Control.Monad.ST.Lazy.Imp.strictToLazy!  We get something like
@@ -386,14 +373,11 @@ Consequences:
   However, it's fine to duplicate a can_fail primop.  That is
   the difference between can_fail and has_side_effects.
 
-
---------------- Summary table ------------------------
             can_fail     has_side_effects
 Discard        YES           YES
 Float in       YES           YES
 Float out      NO            NO
 Duplicate      YES           NO
--------------------------------------------------------
 
 How do we achieve these effects?
 
@@ -411,19 +395,6 @@ Note [primOpOkForSpeculation]
   * The no-duplicate thing is done via primOpIsCheap, by making
     has_side_effects things (very very very) not-cheap!
 
-Note [Aggressive PrimOps]
-~~~~~~~~~~~~~~~~~~~~~~~~~
-We have a static flag opt_AggressivePrimOps, off by default, 
-controlled by -faggressive-primops.  When AggressivePrimOps is
-*off* we keep the old behaviour in which
-  a) we do not float in has_side_effect ops
-  b) we never discard has_side_effect ops as dead code
-I now think that this more conservative behaviour is unnecessary, but
-I'm playing safe and making the conservative behaviour the default.
-
-The static flag lets us try the more aggressive behaviour when we
-want, in case there are mysterious errors.
-
 
 \begin{code}
 primOpHasSideEffects :: PrimOp -> Bool
@@ -433,32 +404,28 @@ primOpCanFail :: PrimOp -> Bool
 #include "primop-can-fail.hs-incl"
 
 primOpOkForSpeculation :: PrimOp -> Bool
-  -- ok-for-speculation means the primop can be let-bound
-  -- and can float in and out freely
-  -- See Note [PrimOp can_fail and has_side_effects]
+  -- See Note [primOpOkForSpeculation and primOpOkForFloatOut]
   -- See comments with CoreUtils.exprOkForSpeculation
 primOpOkForSpeculation op
-  = not (primOpHasSideEffects op || primOpCanFail op)
+  = not (primOpHasSideEffects op || primOpOutOfLine op || primOpCanFail op)
 
 primOpOkForSideEffects :: PrimOp -> Bool
 primOpOkForSideEffects op
   = not (primOpHasSideEffects op)
+\end{code}
 
-primOpIsCheap :: PrimOp -> Bool
-primOpIsCheap op 
-  = not (primOpHasSideEffects op)
-     -- This is vital; see Note [PrimOp can_fail and has_side_effects]
- && not (primOpOutOfLine op)
--- && primOpCodeSize op <= primOpCodeSizeDefault 
-     -- The latter two conditions are a HACK; we should 
-     -- really have a proper property on primops that says
-     -- when they are cheap to execute.  For now we are using
-     -- that the code size is small and not out-of-line.
-     --
-     -- NB that as things stand, array indexing operations
-     -- have default-size code size, and hence will be regarded
-     -- as cheap; we might want to make them more expensive!
 
+Note [primOpIsCheap]
+~~~~~~~~~~~~~~~~~~~~
+@primOpIsCheap@, as used in \tr{SimplUtils.lhs}.  For now (HACK
+WARNING), we just borrow some other predicates for a
+what-should-be-good-enough test.  "Cheap" means willing to call it more
+than once, and/or push it inside a lambda.  The latter could change the
+behaviour of 'seq' for primops that can fail, so we don't treat them as cheap.
+
+\begin{code}
+primOpIsCheap :: PrimOp -> Bool
+primOpIsCheap op = primOpOkForSpeculation op
 -- In March 2001, we changed this to
 --      primOpIsCheap op = False
 -- thereby making *no* primops seem cheap.  But this killed eta
diff --git a/compiler/simplCore/FloatIn.lhs b/compiler/simplCore/FloatIn.lhs
index b7e627f5ba..c0c6478a7b 100644
--- a/compiler/simplCore/FloatIn.lhs
+++ b/compiler/simplCore/FloatIn.lhs
@@ -33,7 +33,6 @@ import Type		( isUnLiftedType )
 import VarSet
 import Util		( zipEqual, zipWithEqual, count )
 import UniqFM
-import StaticFlags      ( opt_AggressivePrimOps )
 import Outputable
 \end{code}
 
@@ -366,14 +365,7 @@ floating in cases with a single alternative that may bind values.
 \begin{code}
 fiExpr to_drop (_, AnnCase scrut case_bndr _ [(con,alt_bndrs,rhs)])
   | isUnLiftedType (idType case_bndr)
-  , opt_AggressivePrimOps || exprOkForSideEffects (deAnnotate scrut)
--- It should be ok to float in ANY primop.
--- See Note [PrimOp can_fail and has_side_effects] in PrimOp
--- The opt_AggressIvePrimOps flag lets us choose between old and new behaviour
--- See Note [Aggressive PrimOps] in PrimOp
---
--- It would NOT be ok if a primop evaluated an unlifted
--- argument, but no primop does that.
+  , exprOkForSideEffects (deAnnotate scrut)
   = wrapFloats shared_binds $
     fiExpr (case_float : rhs_binds) rhs
   where
diff --git a/compiler/simplCore/Simplify.lhs b/compiler/simplCore/Simplify.lhs
index 0b95050e73..335f86a549 100644
--- a/compiler/simplCore/Simplify.lhs
+++ b/compiler/simplCore/Simplify.lhs
@@ -1657,7 +1657,7 @@ check that
 or
         (b) the scrutinee is a variable and 'x' is used strictly
 or
-        (c) 'x' is not used at all and e certainly terminates
+        (c) 'x' is not used at all and e is ok-for-speculation
 
 For the (c), consider
    case (case a ># b of { True -> (p,q); False -> (q,p) }) of
@@ -1794,22 +1794,19 @@ rebuildCase env scrut case_bndr [(_, bndrs, rhs)] cont
               -- The case binder is going to be evaluated later,
               -- and the scrutinee is a simple variable
 
-     || (is_plain_seq && expr_terminates)
+     || (is_plain_seq && ok_for_spec)
               -- Note: not the same as exprIsHNF
 
     elim_unlifted 
-      | is_plain_seq
-      = if opt_AggressivePrimOps then expr_terminates
-        else exprOkForSideEffects scrut
-            -- The entire case is dead, so we can drop it
-            -- But if AggressivePrimOps isn't on, only drop it
-            -- if it has no side effects. See See Note [Aggressive PrimOps] in PrimOp
-      | otherwise = exprOkForSpeculation scrut
+      | is_plain_seq = exprOkForSideEffects scrut
+            -- The entire case is dead, so we can drop it,
+            -- _unless_ the scrutinee has side effects
+      | otherwise    = exprOkForSpeculation scrut
             -- The case-binder is alive, but we may be able
             -- turn the case into a let, if the expression is ok-for-spec
             -- See Note [Case elimination: unlifted case]
 
-    expr_terminates  = exprCertainlyTerminates scrut
+    ok_for_spec      = exprOkForSpeculation scrut
     is_plain_seq     = isDeadBinder case_bndr	-- Evaluation *only* for effect
     strict_case_bndr = isStrictDmd (idDemandInfo case_bndr)