[Git][ghc/ghc][wip/js-staging] JS Backend: Remove FIXMEs

doyougnu (@doyougnu) gitlab at gitlab.haskell.org
Thu Aug 25 22:56:29 UTC 2022



doyougnu pushed to branch wip/js-staging at Glasgow Haskell Compiler / GHC


Commits:
4a9c9af1 by doyougnu at 2022-08-25T18:56:07-04:00
JS Backend: Remove FIXMEs

StgToJS.Apply: Remove FIXMEs

StgToJS.FFI: remove FIXMEs

StgToJS.Expr: remove FIXMEs

StgToJS: Remove FIXMEs

- - - - -


19 changed files:

- compiler/GHC/StgToJS.hs
- compiler/GHC/StgToJS/Apply.hs
- compiler/GHC/StgToJS/Arg.hs
- compiler/GHC/StgToJS/CodeGen.hs
- compiler/GHC/StgToJS/CoreUtils.hs
- compiler/GHC/StgToJS/DataCon.hs
- compiler/GHC/StgToJS/Deps.hs
- compiler/GHC/StgToJS/Expr.hs
- compiler/GHC/StgToJS/ExprCtx.hs
- compiler/GHC/StgToJS/FFI.hs
- compiler/GHC/StgToJS/Heap.hs
- compiler/GHC/StgToJS/Literal.hs
- compiler/GHC/StgToJS/Object.hs
- compiler/GHC/StgToJS/Prim.hs
- compiler/GHC/StgToJS/Printer.hs
- compiler/GHC/StgToJS/Profiling.hs
- compiler/GHC/StgToJS/Regs.hs
- compiler/GHC/StgToJS/Stack.hs
- compiler/GHC/StgToJS/Utils.hs


Changes:

=====================================
compiler/GHC/StgToJS.hs
=====================================
@@ -70,13 +70,6 @@ import GHC.StgToJS.CodeGen
 -- Primitives that are represented as multiple values (Int64#, Word64#, Addr#)
 -- are passed to FFI functions with multiple arguments.
 --
--- FIXME: specify argument order:
---    high then low (Int64#/Word64#)?
---    array then offset(Addr#)?
---    StablePtr#: do we pass the array?
--- FIXME: how do we return them from FFI? With h$retN variables as for
--- unboex tuples?
---
 -- Interruptible convention: FFI imports with the "interruptible" calling
 -- convention are passed an extra argument (usually named "$c") that is a
 -- continuation function. The FFI function must call this function to return to
@@ -128,8 +121,8 @@ import GHC.StgToJS.CodeGen
 --    , i     -- (array) fields layout (empty if variable layout)
 --    , n     -- (string) object name for easier dubugging
 --    , a     -- constructor tag / fun arity
---    , r     -- FIXME
---    , s     -- static references? FIXME
+--    , r     -- ??
+--    , s     -- static references?
 --    , m     -- GC mark?
 --    }
 --


=====================================
compiler/GHC/StgToJS/Apply.hs
=====================================
@@ -107,7 +107,6 @@ genApp ctx i args
     , [top] <- concatMap typex_expr (ctxTarget ctx)
     , getUnique i == unpackCStringAppendIdKey
     , d <- utf8DecodeByteString bs
-        -- FIXME (Sylvain, 2022/02): we assume that it decodes but it may not (e.g. embedded file)
     = do
         prof <- csProf <$> getSettings
         let profArg = if prof then [jCafCCS] else []
@@ -222,7 +221,7 @@ genApp ctx i args
     | n <- length args
     , n /= 0
     , idFunRepArity i == n
-    , not (isLocalId i) -- FIXME (Sylvain 2022-08): why are we testing this here and not in the oversaturated case below?
+    , not (isLocalId i)
     , isStrictId i
     = do
       as' <- concatMapM genArg args
@@ -550,9 +549,6 @@ genericStackApply cfg = closure info body
             , ifS (newAp .===. var "h$ap_gen")
                    ((sp |= sp - needed_regs) <> (stack .! (sp - 1) |= newTag))
                    (sp |= sp - needed_regs - 1)
-                -- FIXME (Sylvain 2022-08): this is fragile and probably inefficient.
-                -- Instead of filling h$apply array with h$ap_gen, we should leave
-                -- it with empty items and match "undefined" here.
 
             -- Push generic application function as continuation
             , stack .! sp |= newAp
@@ -760,17 +756,12 @@ stackApply s fun_name nargs nvars =
                                 (ifS (toJExpr nargs .>. arity)
                                   (traceRts s (toJExpr (fun_name <> ": oversat")) <> oversatCase c arity0 arity)
                                   (traceRts s (toJExpr (fun_name <> ": undersat"))
-                                   <> mkPap s pap r1 (toJExpr nargs) stackArgs -- FIXME do we want double pap?
+                                   <> mkPap s pap r1 (toJExpr nargs) stackArgs
                                    <> (sp |= sp - toJExpr (nvars + 1))
                                    <> (r1 |= toJExpr pap)
                                    <> returnStack))
                               ]
-        _                   -> mempty -- FIXME: Jeff (2022,03), just quieting non-exhaustive
-                                      -- patterns. That the code wants to do this
-                                      -- means we should be encoding that funCase is
-                                      -- only callable on ValExpr (JVar pap)'s in
-                                      -- the type system, perhaps with a GADT or
-                                      -- phantom
+        _                   -> mempty
 
 
     funCase :: JExpr -> JStat
@@ -789,12 +780,7 @@ stackApply s fun_name nargs nvars =
                                   <> (r1 |= toJExpr pap)
                                   <> returnStack))
                               ]
-        _                  -> mempty  -- FIXME: Jeff (2022,03), just quieting non-exhaustive
-                                      -- patterns. That the code wants to do this
-                                      -- means we should be encoding that funCase is
-                                      -- only callable on ValExpr (JVar pap)'s in
-                                      -- the type system, perhaps with a GADT or
-                                      -- phantom
+        _                  -> mempty
 
 
     -- oversat: call the function but keep enough on the stack for the next
@@ -843,7 +829,6 @@ fastApply s fun_name nargs nvars = func ||= body0
         jVar \c farity arity ->
           [ c |= closureEntry r1
           , traceRts s (toJExpr (fun_name <> ": sp ") + sp)
-          -- TODO: Jeff (2022,03): factor our and dry out this code
           , SwitchStat (entryClosureType c)
              [(toJExpr Fun, traceRts s (toJExpr (fun_name <> ": ")
                                         + clName c
@@ -872,12 +857,7 @@ fastApply s fun_name nargs nvars = func ||= body0
                                      <> (r1 |= toJExpr pap)
                                      <> returnStack))
                                 ]
-          _             -> mempty -- FIXME: Jeff (2022,03), just quieting non-exhaustive
-                                  -- patterns. That the code wants to do this
-                                  -- means we should be encoding that funCase is
-                                  -- only callable on ValExpr (JVar pap)'s in
-                                  -- the type system, perhaps with a GADT or
-                                  -- phantom
+          _             -> mempty
 
       oversatCase :: JExpr -> JExpr -> JStat
       oversatCase c arity =
@@ -1130,13 +1110,11 @@ papGen cfg =
 
 -- general utilities
 -- move the first n registers, starting at R2, m places up (do not use with negative m)
--- FIXME (Jeff, 2022/03): pick a better name, e.g., `r2moveRegs`
 moveRegs2 :: JStat
 moveRegs2 = TxtI "h$moveRegs2" ||= jLam moveSwitch
   where
     moveSwitch n m = SwitchStat ((n .<<. 8) .|. m) switchCases (defaultCase n m)
     -- fast cases
-    -- TODO: tune the parameteters for performance and size
     switchCases = [switchCase n m | n <- [1..5], m <- [1..4]]
     switchCase :: Int -> Int -> (JExpr, JStat)
     switchCase n m = (toJExpr $


=====================================
compiler/GHC/StgToJS/Arg.hs
=====================================
@@ -158,7 +158,7 @@ genArg a = case a of
            as <- concat <$> mapM genArg args
            e  <- varForDataConWorker dc
            inl_alloc <- csInlineAlloc <$> getSettings
-           return [allocDynamicE inl_alloc e as Nothing] -- FIXME: ccs
+           return [allocDynamicE inl_alloc e as Nothing]
       x -> pprPanic "genArg: unexpected unfloated expression" (pprStgExpr panicStgPprOpts x)
 
 genIdArg :: HasDebugCallStack => Id -> G [JExpr]
@@ -258,7 +258,6 @@ jsStaticArg = \case
   StaticLitArg l      -> toJExpr l
   StaticObjArg t      -> ValExpr (JVar (TxtI t))
   StaticConArg c args ->
-    -- FIXME: cost-centre stack
     allocDynamicE False (ValExpr . JVar . TxtI $ c) (map jsStaticArg args) Nothing
 
 -- | Generate JS code corresponding to a list of static args


=====================================
compiler/GHC/StgToJS/CodeGen.hs
=====================================
@@ -138,7 +138,7 @@ genUnits m ss spt_entries foreign_stubs
         staticInit <-
           initStaticPtrs spt_entries
         (st', _, bs) <- serializeLinkableUnit m st [] [] []
-                         ( -- FIXME (Sylvain, 2022/02): optimizer disabled: O.optimize .
+                         ( -- O.optimize .
                            jsSaturate (Just $ modulePrefix m 1)
                          $ mconcat (reverse glbl) <> staticInit) "" [] []
         return ( st'
@@ -218,8 +218,7 @@ genUnits m ss spt_entries foreign_stubs
         let allDeps  = collectIds unf decl
             topDeps  = collectTopIds decl
             required = hasExport decl
-            stat     = -- FIXME (Sylvain 2022/02): optimizer disabled:
-                       -- {-decl -} Opt.optimize .
+            stat     = -- Opt.optimize .
                        jsSaturate (Just $ modulePrefix m n)
                      $ mconcat (reverse extraTl) <> tl
         (st', _ss, bs) <- serializeLinkableUnit m st topDeps ci si stat mempty [] fRefs


=====================================
compiler/GHC/StgToJS/CoreUtils.hs
=====================================
@@ -203,9 +203,9 @@ primTypeVt t = case tyConAppTyCon_maybe (unwrapType t) of
     | tc == mutVarPrimTyCon            -> RtsObjV
     | tc == mVarPrimTyCon              -> RtsObjV
     | tc == tVarPrimTyCon              -> RtsObjV
-    | tc == bcoPrimTyCon               -> RtsObjV -- fixme what do we need here?
+    | tc == bcoPrimTyCon               -> RtsObjV -- unsupported?
     | tc == stackSnapshotPrimTyCon     -> RtsObjV
-    | tc == ioPortPrimTyCon            -> RtsObjV -- FIXME: Jeff (2022, 05) IOPort, how to handle in JS?
+    | tc == ioPortPrimTyCon            -> RtsObjV -- unsupported?
     | tc == anyTyCon                   -> PtrV
     | tc == compactPrimTyCon           -> ObjV -- unsupported?
     | tc == eqPrimTyCon                -> VoidV -- coercion token?


=====================================
compiler/GHC/StgToJS/DataCon.hs
=====================================
@@ -43,9 +43,6 @@ genCon ctx con args
   | [ValExpr (JVar ctxi)] <- concatMap typex_expr (ctxTarget ctx)
   = allocCon ctxi con currentCCS args
 
-  -- FIXME: (Sylvain 2022-03-11) Do we support e.g. "data T = MkT Word64"? It
-  -- would return two JExprs
-
   | xs <- concatMap typex_expr (ctxTarget ctx)
   = pprPanic "genCon: unhandled DataCon" (ppr (con, args, xs))
 


=====================================
compiler/GHC/StgToJS/Deps.hs
=====================================
@@ -39,7 +39,7 @@ import Control.Monad
 import Control.Monad.Trans.Class
 import Control.Monad.Trans.State
 
-data DependencyDataCache = DDC -- FIXME Sylvain 2022-02: use UniqFM
+data DependencyDataCache = DDC
   { ddcModule :: !(IntMap Unit)               -- ^ Unique Module -> Object.Package
   , ddcId     :: !(IntMap Object.ExportedFun) -- ^ Unique Id     -> Object.ExportedFun (only to other modules)
   , ddcOther  :: !(Map OtherSymb Object.ExportedFun)


=====================================
compiler/GHC/StgToJS/Expr.hs
=====================================
@@ -331,18 +331,10 @@ resultSize xxs@(_:xs) t
   | otherwise = [(LiftedRep, 1)] -- possibly newtype family, must be boxed
 
 resultSize [] t
-  -- FIXME: Jeff (2022,05): Is this check actually needed? If we have a runtime
-  -- rep kinded type can't we just call typePrimReps to get the PrimReps and
-  -- then primRep size just like in the catchall case? I don't see why this
-  -- doesn't work.
   | isRuntimeRepKindedTy t' = pprPanic "resultSize: Type was RuntimeRepKinded don't know the size! " (ppr t')
-
   -- Note that RuntimeRep from Builtins.Types hits this case. A singleton of
   -- (LiftedRep, 1) is exactly what's returned by the otherwise case for
   -- RuntimeRep.
-  -- FIXME: Luite (2022,07): typeLevity_maybe can panic, doesn't the next case
-  -- give us the right answer?
-  --  Nothing <- typeLevity_maybe t' = [(LiftedRep, 1)]
   | otherwise = fmap (\p -> (p, slotCount (primRepSize p))) (typePrimReps t)
   where
     t' = unwrapType t
@@ -403,9 +395,6 @@ popLneFrame inEntry size ctx = do
   let ctx' = ctxLneShrinkStack ctx size
 
   let gen_id_slot (i,n) = do
-        -- FIXME (Sylvain 2022-08): do we really need to generate all the Idents here
-        -- to only select one? Is it because we need the side effect that consists in
-        -- filling the GlobalId cache?
         ids <- identsForId i
         let !id_n = ids !! (n-1)
         pure (id_n, SlotId i n)
@@ -698,8 +687,6 @@ genAlts ctx e at me alts = do
             return (s, r)
           _ -> error "genAlts: invalid branches for Bool"
 
-    -- FIXME: add all alts
-
     AlgAlt _tc -> do
         ei <- varForId e
         (r, brs) <- normalizeBranches ctx <$>
@@ -923,11 +910,7 @@ allocDynAll haveDecl middle cls = do
 
     fillObjs = mconcat $ map fillObj cls
     fillObj (i,_,es,_)
-      | csInlineAlloc settings || length es > 24 = -- FIXME (Jeff, 2022/03): the call to length means `es`
-                                                   -- should be something other than
-                                                   -- a list. Also why is 24
-                                                   -- important? And 24 should be a
-                                                   -- constant such as `fooThreshold`
+      | csInlineAlloc settings || length es > 24 =
           case es of
             []      -> mempty
             [ex]    -> toJExpr i .^ closureField1_ |= toJExpr ex


=====================================
compiler/GHC/StgToJS/ExprCtx.hs
=====================================
@@ -127,7 +127,6 @@ ctxUpdateLneFrame new_spilled_vars new_lne_ids ctx =
     { ctxLneFrameBs   = addListToUFM (ctxLneFrameBs ctx) (map (,new_frame_size) new_lne_ids)
     , ctxLneFrameSize = new_frame_size
     , ctxLneFrameVars = ctxLneFrameVars ctx ++ new_spilled_vars
-                          -- FIXME: could we use a stack? (i.e. cons new variables)
     }
 
 -- | Remove information about the current LNE frame


=====================================
compiler/GHC/StgToJS/FFI.hs
=====================================
@@ -50,7 +50,6 @@ import Control.Monad
 import Control.Applicative
 import qualified Text.ParserCombinators.ReadP as P
 
--- FIXME: what if the call returns a thunk?
 genPrimCall :: ExprCtx -> PrimCall -> [StgArg] -> Type -> G (JStat, ExprResult)
 genPrimCall ctx (PrimCall lbl _) args t = do
   j <- parseFFIPattern False False False ("h$" ++ unpackFS lbl) t (concatMap typex_expr $ ctxTarget ctx) args
@@ -140,8 +139,7 @@ parseFFIPattern' :: Maybe JExpr -- ^ Nothing for sync, Just callback for async
 parseFFIPattern' callback javascriptCc pat t ret args
   | not javascriptCc = mkApply pat
   | otherwise =
-   if True -- FIXME (Sylvain 2022-03): we don't support parsing of JS imports.
-           -- So we assume that we can directly apply to them...
+   if True
      then mkApply pat
      else do
       u <- freshUnique
@@ -278,13 +276,9 @@ callbackPlaceholders (Just e) = [((TxtI "$c"), e)]
 
 parseFfiJME :: String -> Int -> Either String JExpr
 parseFfiJME _xs _u =  Left "parseFfiJME not yet implemented"
-  -- FIXME (Sylvain 2022-02): removed temporarily for the codegen merge. Need to
-  -- decide which syntax we support
 
 parseFfiJM :: String -> Int -> Either String JStat
 parseFfiJM _xs _u = Left "parseFfiJM not yet implemented"
-  -- FIXME (Sylvain 2022-02): removed temporarily for the codegen merge. Need to
-  -- decide which syntax we support
 
 saturateFFI :: JMacro a => Int -> a -> a
 saturateFFI u = jsSaturate (Just . mkFastString $ "ghcjs_ffi_sat_" ++ show u)


=====================================
compiler/GHC/StgToJS/Heap.hs
=====================================
@@ -43,21 +43,6 @@ import GHC.JS.Make
 import GHC.StgToJS.Types
 import GHC.Data.FastString
 
--- FIXME: Jeff (2022,03): These helpers are a classic case of using a newtype
--- over a type synonym to leverage GHC's type checker. Basically we never want
--- to mix these up, and so we should have:
---------------------------------------
--- newtype ClosureEntry  = ClosureEntry  { unClosureEntry  :: FastString }
--- newtype ClosureExtra1 = ClosureExtra1 { unClosureExtra1 :: FastString }
--- newtype ClosureExtra2 = ClosureExtra2 { unClosureExtra2 :: FastString }
--- newtype ClosureMeta   = ClosureMeta   { unClosureMeta   :: FastString }
---------------------------------------
--- especially since any bugs which result from confusing these will be catastrophic and hard to debug
--- also NOTE: if ClosureExtra<N> is truly unbounded then we should have:
--- newtype ClosureExtras = ClosureExtras { unClosureExtras :: [FastString] }
--- or use an Array and amortize increasing the arrays size when needed; depending
--- on its use case in the RTS of course
-
 closureEntry_ :: FastString
 closureEntry_ = "f"
 


=====================================
compiler/GHC/StgToJS/Literal.hs
=====================================
@@ -73,12 +73,6 @@ genStaticLit = \case
   LitChar c                -> return [ IntLit (fromIntegral $ ord c) ]
   LitString str
     | True                 -> return [ StringLit (mkFastStringByteString str), IntLit 0]
-    -- FIXME: documentation for LitString says it's always UTF8 encoded but it's
-    -- not true (e.g. for embedded files).
-    --  1) We should add a decoding function that detects errors in
-    --  GHC.Utils.Encoding
-    --  2) We should perhaps add a different LitBin constructor that would
-    --  benefit other backends?
     -- \|  invalid UTF8         -> return [ BinLit str, IntLit 0]
   LitNullAddr              -> return [ NullLit, IntLit 0 ]
   LitNumber nt v           -> case nt of
@@ -97,7 +91,6 @@ genStaticLit = \case
   LitDouble r              -> return [ DoubleLit . SaneDouble . r2d $ r ]
   LitLabel name _size fod  -> return [ LabelLit (fod == IsFunction) (mkFastString $ "h$" ++ unpackFS name)
                                      , IntLit 0 ]
-  -- FIXME: handle other LitRubbish, etc.
   l -> pprPanic "genStaticLit" (ppr l)
 
 -- make an unsigned 32 bit number from this unsigned one, lower 32 bits


=====================================
compiler/GHC/StgToJS/Object.hs
=====================================
@@ -6,7 +6,7 @@
 {-# LANGUAGE ScopedTypeVariables        #-}
 {-# LANGUAGE TupleSections              #-}
 
--- only for DB.Binary instances on Module see FIXME below
+-- only for DB.Binary instances on Module
 {-# OPTIONS_GHC -fno-warn-orphans #-}
 
 -----------------------------------------------------------------------------
@@ -36,16 +36,7 @@
 --   - dependency info
 --   - closureinfo index
 --   - closureinfo data (offsets described by index)
-
--- FIXME: Jeff (2022,03): There are orphan instances for DB.Binary Module and
--- ModuleName. These are needed in StgToJS.Linker.Types for @Base@ serialization
--- in @putBase at . We end up in this situation because Base now holds a @Module@
--- type instead of GHCJS's previous @Package@ type. In addition to this GHC uses
--- GHC.Utils.Binary for binary instances rather than Data.Binary (even though
--- Data.Binary is a boot lib) so to fix the situation we must:
--- - 1. Choose to use GHC.Utils.Binary or Data.Binary
--- - 2. Remove Binary since this is redundant
--- - 3. Adapt the Linker types, like Base to the new Binary methods
+--
 -----------------------------------------------------------------------------
 
 module GHC.StgToJS.Object
@@ -174,7 +165,6 @@ trim = let f = dropWhile isSpace . reverse in f . f
 isGlobalUnit :: Int -> Bool
 isGlobalUnit n = n == 0
 
--- fixme document, exports unit is always linked
 isExportsUnit :: Int -> Bool
 isExportsUnit n = n == 1
 
@@ -492,9 +482,6 @@ putSymbolTable (SymbolTable _ hm) = st
       st = DB.runPut $ do
               DB.putWord32le (fromIntegral $ length xs)
               mapM_ DB.put xs
-              -- fixme: this is a workaround for some weird issue sometimes causing zero-length
-              --        strings when using the Data.Text instance directly
-              -- mapM_ (DB.put . TE.encodeUtf8) xs
       xs :: [FastString]
       xs = map fst . sortBy (compare `on` snd) . nonDetEltsUniqMap $ hm
       -- We can use `nonDetEltsUniqMap` because the paired `Int`s introduce ordering.


=====================================
compiler/GHC/StgToJS/Prim.hs
=====================================
@@ -578,7 +578,6 @@ genPrim prof ty op = case op of
                                                                     , a .! i |= new
                                                                     , s |= zero_
                                                                     ]
-                                                                    -- fixme both new?
                                                                     [ s |= one_
                                                                     , o |= x
                                                                     ]
@@ -705,7 +704,6 @@ genPrim prof ty op = case op of
       PrimInline $ r |= app "h$compareByteArrays" [a1,o1,a2,o2,n]
 
   CopyByteArrayOp -> \[] [a1,o1,a2,o2,n] ->
-      -- FIXME: we can do faster by copying 32 bit ints or doubles
       PrimInline $ loopBlockS (Sub n one_) (.>=. zero_) \i ->
         [ u8_ a2 (Add i o2) |= u8_ a1 (Add i o1)
         , postDecrS i
@@ -742,7 +740,7 @@ genPrim prof ty op = case op of
   AddrSubOp   -> \[i]     [_a1,o1,_a2,o2] -> PrimInline $ i |= Sub o1 o2
   AddrRemOp   -> \[r]     [_a,o,i]        -> PrimInline $ r |= Mod o i
   AddrToIntOp -> \[i]     [_a,o]          -> PrimInline $ i |= o -- only usable for comparisons within one range
-  IntToAddrOp -> \[a,o]   [i]             -> PrimInline $ mconcat [a |= null_, o |= i] -- FIXME: unsupported
+  IntToAddrOp -> \[a,o]   [i]             -> PrimInline $ mconcat [a |= null_, o |= i]
   AddrGtOp -> \[r] [a1,o1,a2,o2] -> PrimInline $ r |= if10 (app "h$comparePointer" [a1,o1,a2,o2] .>. zero_)
   AddrGeOp -> \[r] [a1,o1,a2,o2] -> PrimInline $ r |= if10 (app "h$comparePointer" [a1,o1,a2,o2] .>=. zero_)
   AddrEqOp -> \[r] [a1,o1,a2,o2] -> PrimInline $ r |= if10 (app "h$comparePointer" [a1,o1,a2,o2] .===. zero_)
@@ -920,13 +918,13 @@ genPrim prof ty op = case op of
 
   MkWeakOp              -> \[r] [o,b,c] -> PrimInline $ r |= app "h$makeWeak" [o,b,c]
   MkWeakNoFinalizerOp   -> \[r] [o,b]   -> PrimInline $ r |= app "h$makeWeakNoFinalizer" [o,b]
-  AddCFinalizerToWeakOp -> \[r] [_a1,_a1o,_a2,_a2o,_i,_a3,_a3o,_w] -> PrimInline $ r |= one_ -- fixme
+  AddCFinalizerToWeakOp -> \[r] [_a1,_a1o,_a2,_a2o,_i,_a3,_a3o,_w] -> PrimInline $ r |= one_
   DeRefWeakOp           -> \[f,v] [w] -> PrimInline $ mconcat
                                                         [ v |= w .^ "val"
                                                         , f |= if01 (v .===. null_)
                                                         ]
   FinalizeWeakOp     -> \[fl,fin] [w] -> PrimInline $ appT [fin, fl] "h$finalizeWeak" [w]
-  TouchOp            -> \[] [_e]      -> PrimInline mempty -- fixme what to do?
+  TouchOp            -> \[] [_e]      -> PrimInline mempty
   KeepAliveOp        -> \[_r] [x, f]  -> PRPrimCall $ ReturnStat (app "h$keepAlive" [x, f])
 
 


=====================================
compiler/GHC/StgToJS/Printer.hs
=====================================
@@ -63,8 +63,6 @@ ghcjsRenderJsV r (JHash m)
     quoteIfRequired x
       | isUnquotedKey x' = text x'
       | otherwise        = PP.squotes (text x')
-    -- FIXME: Jeff (2022,03): remove the deserialization to String. We are only
-    -- converting from ShortText to String here to call @all@ and @tail at .
       where x' = unpackFS x
 
     isUnquotedKey :: String -> Bool
@@ -74,7 +72,6 @@ ghcjsRenderJsV r (JHash m)
                                       && all validOtherIdent (tail x)
 
 
-    -- fixme, this will quote some idents that don't really need to be quoted
     validFirstIdent c = c == '_' || c == '$' || isAlpha c
     validOtherIdent c = isAlpha c || isDigit c
 ghcjsRenderJsV r v = renderJsV defaultRenderJs r v
@@ -119,7 +116,7 @@ prettyBlock' r ( (DeclStat i)
                )
       | i == i' = (text "var" <+> jsToDocR r i <+> char '=' <+> jsToDocR r v) : prettyBlock' r xs
 
--- modify/assign operators (fixme this should be more general, but beware of side effects like PPostExpr)
+-- modify/assign operators
 prettyBlock' r ( (AssignStat (ValExpr (JVar i)) (InfixExpr AddOp (ValExpr (JVar i')) (ValExpr (JInt 1))))
                : xs
                )


=====================================
compiler/GHC/StgToJS/Profiling.hs
=====================================
@@ -88,7 +88,6 @@ enterCostCentreThunk :: JStat
 enterCostCentreThunk = ApplStat (var "h$enterThunkCCS") [r1 .^ "cc"]
 
 setCC :: CostCentre -> Bool -> Bool -> G JStat
--- FIXME: ignoring tick flags for now
 setCC cc _tick True = do
   ccI@(TxtI _ccLbl) <- costCentreLbl cc
   addDependency $ OtherSymb (cc_mod cc)
@@ -147,7 +146,7 @@ costCentreStackLbl' :: CostCentreStack -> G (Maybe String)
 costCentreStackLbl' ccs = do
   ifProfilingM f
   where
-    f | isCurrentCCS ccs   = return $ Just "h$currentThread.ccs" -- FIXME
+    f | isCurrentCCS ccs   = return $ Just "h$currentThread.ccs"
       | dontCareCCS == ccs = return $ Just "h$CCS_DONT_CARE"
       | otherwise          =
           case maybeSingletonCCS ccs of


=====================================
compiler/GHC/StgToJS/Regs.hs
=====================================
@@ -29,18 +29,6 @@ import GHC.Data.FastString
 import Data.Array
 import Data.Char
 
--- FIXME: Perf: Jeff (2022,03): as far as I can tell, we never pattern match on
--- these registers and make heavy use of the Enum, Bounded, and Ix, instances.
--- This heavily implies to me that we should be using something like: StgReg =
--- StgReg { unStgReg :: Int8# } and then store two nibbles in a single byte. Not
--- only would this be more memory efficient, but it would also allow for
--- optimizations such as pointer tagging and avoiding chasing the info table,
--- although I'm not sure if this would really benefit the backend as currently
--- written. Other than that a newtype wrapper with a custom bounded instance
--- (hand written or deriving via) would be better. In almost all functions that
--- take an StgReg we use either the Bounded or the Enum methods, thus we likely
--- don't gain anything from having these registers explicitly represented in
--- data constructors.
 -- | General purpose "registers"
 --
 -- The JS backend arbitrarily supports 128 registers
@@ -103,8 +91,6 @@ r3 = toJExpr R3
 r4 = toJExpr R4
 
 
--- FIXME: Jeff (2022,03): remove these serialization functions after adding a
--- StgReg type with a proper bounded and enum instance
 jsRegToInt :: StgReg -> Int
 jsRegToInt = (+1) . fromEnum
 


=====================================
compiler/GHC/StgToJS/Stack.hs
=====================================
@@ -182,9 +182,6 @@ pushOptimized' xs = do
   pushOptimized =<< (zipWithM f xs (slots++repeat SlotUnknown))
   where
     f (i1,n1) xs2 = do
-      -- FIXME (Sylvain 2022-08): do we really need to generate all the Idents here
-      -- to only select one? Is it because we need the side effect that consists in
-      -- filling the GlobalId cache?
       xs <- varsForId i1
       let !id_n1 = xs !! (n1-1)
 


=====================================
compiler/GHC/StgToJS/Utils.hs
=====================================
@@ -52,7 +52,6 @@ assignCoerce (TypedExpr AddrRep [a_val, a_off]) (TypedExpr UnliftedRep [sptr]) =
     , a_off |= sptr
     ]
 assignCoerce (TypedExpr UnliftedRep [sptr]) (TypedExpr AddrRep [_a_val, a_off]) =
-  -- FIXME: (Sylvain 2022-03-11): why can we ignore a_val?
   sptr |= a_off
 assignCoerce p1 p2 = assignTypedExprs [p1] [p2]
 



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

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/4a9c9af11f79586037b6f7dfd2560e4b1587cdf2
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/20220825/634a23fb/attachment-0001.html>


More information about the ghc-commits mailing list