[Git][ghc/ghc][wip/T8317] Remove a dead comment

Simon Peyton Jones (@simonpj) gitlab at gitlab.haskell.org
Wed Feb 7 17:45:26 UTC 2024



Simon Peyton Jones pushed to branch wip/T8317 at Glasgow Haskell Compiler / GHC


Commits:
4dfdc96b by Simon Peyton Jones at 2024-02-07T17:44:23+00:00
Remove a dead comment

Just remove an out of date block of commented-out code, and tidy up
the relevant Notes.  See #8317.

- - - - -


4 changed files:

- compiler/GHC/Core/Opt/ConstantFold.hs
- compiler/GHC/Core/Opt/Simplify/Iteration.hs
- compiler/GHC/StgToCmm/Expr.hs
- testsuite/tests/linters/notes.stdout


Changes:

=====================================
compiler/GHC/Core/Opt/ConstantFold.hs
=====================================
@@ -3518,7 +3518,7 @@ tx_con_dtt _ alt = pprPanic "caseRules/dataToTag: bad alt" (ppr alt)
 {- Note [caseRules for tagToEnum]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 We want to transform
-   case tagToEnum x of
+   case tagToEnum# x of
      False -> e1
      True  -> e2
 into
@@ -3526,13 +3526,13 @@ into
      0# -> e1
      1# -> e2
 
-This rule eliminates a lot of boilerplate. For
+See #8317.   This rule eliminates a lot of boilerplate. For
   if (x>y) then e2 else e1
 we generate
-  case tagToEnum (x ># y) of
+  case tagToEnum# (x ># y) of
     False -> e1
     True  -> e2
-and it is nice to then get rid of the tagToEnum.
+and it is nice to then get rid of the tagToEnum#.
 
 Beware (#14768): avoid the temptation to map constructor 0 to
 DEFAULT, in the hope of getting this
@@ -3550,8 +3550,9 @@ We don't want to get this!
       DEFAULT -> e1
       DEFAULT -> e2
 
-Instead, we deal with turning one branch into DEFAULT in GHC.Core.Opt.Simplify.Utils
-(add_default in mkCase3).
+Instead, when possible, we turn one branch into DEFAULT in
+GHC.Core.Opt.Simplify.Utils.mkCase2; see Note [Literal cases]
+in that module.
 
 Note [caseRules for dataToTag]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


=====================================
compiler/GHC/Core/Opt/Simplify/Iteration.hs
=====================================
@@ -2510,27 +2510,6 @@ tryRules env rules fn args call_cont
   | null rules
   = return Nothing
 
-{- Disabled until we fix #8326
-  | fn `hasKey` tagToEnumKey   -- See Note [Optimising tagToEnum#]
-  , [_type_arg, val_arg] <- args
-  , Select dup bndr ((_,[],rhs1) : rest_alts) se cont <- call_cont
-  , isDeadBinder bndr
-  = do { let enum_to_tag :: CoreAlt -> CoreAlt
-                -- Takes   K -> e  into   tagK# -> e
-                -- where tagK# is the tag of constructor K
-             enum_to_tag (DataAlt con, [], rhs)
-               = assert (isEnumerationTyCon (dataConTyCon con) )
-                (LitAlt tag, [], rhs)
-              where
-                tag = mkLitInt (sePlatform env) (toInteger (dataConTag con - fIRST_TAG))
-             enum_to_tag alt = pprPanic "tryRules: tagToEnum" (ppr alt)
-
-             new_alts = (DEFAULT, [], rhs1) : map enum_to_tag rest_alts
-             new_bndr = setIdType bndr intPrimTy
-                 -- The binder is dead, but should have the right type
-      ; return (Just (val_arg, Select dup new_bndr new_alts se cont)) }
--}
-
   | Just (rule, rule_rhs) <- lookupRule ropts (getUnfoldingInRuleMatch env)
                                         (activeRule (seMode env)) fn
                                         (argInfoAppArgs args) rules
@@ -2706,25 +2685,6 @@ Note [OccInfo in unfoldings and rules] in GHC.Core.  There is something
 unsatisfactory about doing it twice; but the rule RHS is usually very
 small, and this is simple.
 
-Note [Optimising tagToEnum#]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-If we have an enumeration data type:
-
-  data Foo = A | B | C
-
-Then we want to transform
-
-   case tagToEnum# x of   ==>    case x of
-     A -> e1                       DEFAULT -> e1
-     B -> e2                       1#      -> e2
-     C -> e3                       2#      -> e3
-
-thereby getting rid of the tagToEnum# altogether.  If there was a DEFAULT
-alternative we retain it (remember it comes first).  If not the case must
-be exhaustive, and we reflect that in the transformed version by adding
-a DEFAULT.  Otherwise Lint complains that the new case is not exhaustive.
-See #8317.
-
 Note [Rules for recursive functions]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 You might think that we shouldn't apply rules for a loop breaker:


=====================================
compiler/GHC/StgToCmm/Expr.hs
=====================================
@@ -234,6 +234,8 @@ start of each alternative.  Of course we certainly have to do so if
 the case forces an evaluation, or if there is a primitive op which can
 trigger GC.
 
+NB: things are not settled here: see #8326.
+
 A more interesting situation is this (a Plan-B situation)
 
         !P!;
@@ -652,20 +654,10 @@ cgCase scrut bndr alt_type alts
 
 {- Note [GC for conditionals]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-For boolean conditionals it seems that we have always done NoGcInAlts.
-That is, we have always done the GC check before the conditional.
-This is enshrined in the special case for
-   case tagToEnum# (a>b) of ...
-See Note [case on bool]
-
-It's odd, and it's flagrantly inconsistent with the rules described
-Note [Compiling case expressions].  However, after eliminating the
-tagToEnum# (#13397) we will have:
-   case (a>b) of ...
-Rather than make it behave quite differently, I am testing for a
-comparison operator here in the general case as well.
-
-ToDo: figure out what the Right Rule should be.
+For comparison operators (`is_cmp_op`) it seems that we have always done
+NoGcInAlts.  It's odd, and it's flagrantly inconsistent with the rules described
+Note [Compiling case expressions].  However, that's the way it has been for ages
+(there was some long-gone history involving tagToEnum#; see #13397, #8317, #8326).
 
 Note [scrut sequel]
 ~~~~~~~~~~~~~~~~~~~


=====================================
testsuite/tests/linters/notes.stdout
=====================================
@@ -16,7 +16,6 @@ ref    compiler/GHC/Hs/Pat.hs:141:74:     Note [Lifecycle of a splice]
 ref    compiler/GHC/HsToCore/Pmc/Solver.hs:856:20:     Note [COMPLETE sets on data families]
 ref    compiler/GHC/HsToCore/Quote.hs:1487:7:     Note [How brackets and nested splices are handled]
 ref    compiler/GHC/Stg/Unarise.hs:438:32:     Note [Renaming during unarisation]
-ref    compiler/GHC/StgToCmm/Expr.hs:578:4:     Note [case on bool]
 ref    compiler/GHC/Tc/Gen/HsType.hs:556:56:     Note [Skolem escape prevention]
 ref    compiler/GHC/Tc/Gen/HsType.hs:2676:7:     Note [Matching a kind signature with a declaration]
 ref    compiler/GHC/Tc/Gen/Pat.hs:174:20:     Note [Typing patterns in pattern bindings]



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/4dfdc96bf16f7d7c3edab13bb75a3549f36f7ab7

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/4dfdc96bf16f7d7c3edab13bb75a3549f36f7ab7
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20240207/801c0c42/attachment-0001.html>


More information about the ghc-commits mailing list