Simon Jakobi pushed to branch wip/sjakobi/T27628-rebox-warning at Glasgow Haskell Compiler / GHC

Commits:

3 changed files:

Changes:

  • compiler/GHC/Core/Opt/SpecConstr.hs
    ... ... @@ -18,7 +18,8 @@ module GHC.Core.Opt.SpecConstr(
    18 18
    19 19 
     import GHC.Prelude
    20 20
    21 
    -import GHC.Driver.DynFlags ( DynFlags(..), GeneralFlag( Opt_SpecConstrKeen )
    21 
    +import GHC.Driver.DynFlags ( DynFlags(..)
    22 
    +                          , GeneralFlag( Opt_SpecConstrKeen, Opt_SuppressUniques )
    22 23 
                               , WarningFlag( Opt_WarnSpecConstrReboxing )
    23 24 
                               , gopt, hasPprDebug )
    24 25
    ... ... @@ -61,7 +62,7 @@ import GHC.Types.Demand
    61 62 
     import GHC.Types.Cpr
    62 63 
     import GHC.Types.Unique.Supply
    63 64 
     import GHC.Types.Unique.FM
    64 
    -import GHC.Types.Unique( hasKey )
    65 
    +import GHC.Types.Unique( hasKey, pprUniqueAlways )
    65 66
    66 67 
     import GHC.Data.Maybe     ( fromMaybe, orElse, catMaybes, isJust, isNothing )
    67 68 
     import GHC.Data.FastString
    ... ... @@ -785,6 +786,7 @@ unbox the strict fields, because T is polymorphic!)
    785 786 
     specConstrProgram :: ModGuts -> CoreM ModGuts
    786 787 
     specConstrProgram guts
    787 788 
       = do { env0 <- initScEnv guts
    789 
    +       ; dflags <- getDynFlags
    788 790 
            ; us   <- getUniqueSupplyM
    789 791 
            ; let (_usg, binds', warnings) = initUs_ us $
    790 792 
                                   scTopBinds env0 (mg_binds guts)
    ... ... @@ -795,7 +797,7 @@ specConstrProgram guts
    795 797
    796 798 
            ; when (not (null forced_ws)) $ diagnostic WarningWithoutFlag (forced_msg forced_ws)
    797 799 
            ; mapM_ (diagnostic (WarningWithFlag Opt_WarnSpecConstrReboxing) . rebox_msg)
    798 
    -               (aggregateRebox rebox_ws)
    800 
    +               (aggregateRebox (gopt Opt_SuppressUniques dflags) rebox_ws)
    799 801
    800 802 
            ; return (guts { mg_binds = binds' }) }
    801 803
    ... ... @@ -806,11 +808,13 @@ specConstrProgram guts
    806 808 
                             nest 2 (vcat (map ppr warnings)) $$
    807 809 
                             (text "If this is expected you might want to increase -fmax-forced-spec-args to force specialization anyway.")
    808 810
    809 
    -    -- One warning per specialised function (all its patterns listed),
    810 
    -    -- then warnings that would render identically merged too; see
    811 
    -    -- Note [Reboxing warning]
    812 
    -    aggregateRebox :: SpecConstrWarnings -> SpecConstrWarnings
    813 
    -    aggregateRebox = mergeBy same_render . mergeBy same_fn
    811 
    +    -- One warning per specialised function (all its patterns listed).
    812 
    +    -- Under -dsuppress-uniques, warnings that would render identically
    813 
    +    -- are merged too; see Note [Reboxing warning]
    814 
    +    aggregateRebox :: Bool -> SpecConstrWarnings -> SpecConstrWarnings
    815 
    +    aggregateRebox uniqs_suppressed
    816 
    +      | uniqs_suppressed = mergeBy same_render . mergeBy same_fn
    817 
    +      | otherwise        = mergeBy same_fn
    814 818 
           where
    815 819 
             mergeBy eq ws
    816 820 
               = [ SpecReboxed fn ty parent recur (nubBy same_pat (concat patss)) (nub (concat callerss))
    ... ... @@ -828,8 +832,10 @@ specConstrProgram guts
    828 832 
             -- Merge warnings that would render identically: same occurrence
    829 833 
             -- name, type, parent, displayed location, recursivity, and
    830 834 
             -- patterns (with their spec signatures). The reader could not
    831 
    -        -- tell them apart, so printing both is noise; see
    832 
    -        -- Note [Reboxing warning].  Callers are aggregated, not compared.
    835 
    +        -- tell them apart, so printing both is noise.  Only applied under
    836 
    +        -- -dsuppress-uniques: with uniques shown, distinct copies render
    837 
    +        -- distinctly; see Note [Reboxing warning].  Callers are
    838 
    +        -- aggregated, not compared.
    833 839 
             same_render (SpecReboxed fn1 ty1 p1 r1 pats1 _) (SpecReboxed fn2 ty2 p2 r2 pats2 _)
    834 840 
               = getOccName fn1 == getOccName fn2 && p1 == p2
    835 841 
                 && nameSrcSpan (rebox_loc_name fn1 p1) == nameSrcSpan (rebox_loc_name fn2 p2)
    ... ... @@ -841,10 +847,16 @@ specConstrProgram guts
    841 847 
             same_render _ _ = False
    842 848
    843 849 
             -- Spec signatures of merge candidates are alpha-equivalent copies,
    844 
    -        -- so eqType; a mismatch just leaves two warnings unmerged
    845 
    -        same_pat p1@(ReboxedPat _ _ occ1 sty1) p2@(ReboxedPat _ _ occ2 sty2)
    850 
    +        -- so eqType; a mismatch just leaves two warnings unmerged.  Spec
    851 
    +        -- names compare as displayed: by occurrence name only when the
    852 
    +        -- uniques are suppressed
    853 
    +        same_pat p1@(ReboxedPat _ _ nm1 sty1) p2@(ReboxedPat _ _ nm2 sty2)
    846 854 
               = cmpReboxedPat p1 p2 == EQ
    847 
    -            && occ1 == occ2 && sty1 `eqType` sty2
    855 
    +            && same_disp_name nm1 nm2 && sty1 `eqType` sty2
    856 
    +
    857 
    +        same_disp_name n1 n2
    858 
    +          | uniqs_suppressed = getOccName n1 == getOccName n2
    859 
    +          | otherwise        = n1 == n2
    848 860
    849 861 
             -- Recursivity as displayed: siblings compare by occurrence name,
    850 862 
             -- so span-less copies of one mutual group still merge
    ... ... @@ -862,12 +874,23 @@ specConstrProgram guts
    862 874 
           | not (isGoodSrcSpan (nameSrcSpan fn)) = parent
    863 875 
         rebox_loc_name fn _ = fn
    864 876
    877 
    +    -- Local binders display occ plus unique, as pre-tidy dumps print
    878 
    +    -- them, so the binder can be grepped in -ddump-spec-constr output
    879 
    +    -- of the same compilation; see Note [Reboxing warning].
    880 
    +    -- -dsuppress-uniques hides the unique.
    881 
    +    pp_name :: Name -> SDoc
    882 
    +    pp_name n
    883 
    +      | isExternalName n = ppr n
    884 
    +      | otherwise        = ppr (getOccName n)
    885 
    +                           <> ppUnlessOption sdocSuppressUniques
    886 
    +                                (char '_' <> pprUniqueAlways (nameUnique n))
    887 
    +
    865 888 
         -- See Note [Reboxing warning]
    866 889 
         rebox_msg :: SpecConstrWarning -> SDoc
    867 890 
         rebox_msg w@(SpecFailForcedArgCount {}) = pprPanic "rebox_msg" (ppr w)
    868 891 
         rebox_msg (SpecReboxed fn ty mb_parent recur pats callers)
    869 892 
           = vcat [ hang (text "SpecConstr specialised") 2
    870 
    -                    (quotes (ppr fn <+> dcolon <+> pp_ty ty))
    893 
    +                    (quotes (pp_name fn <+> dcolon <+> pp_ty ty))
    871 894 
                  , nest 2 $ vcat $ catMaybes
    872 895 
                      [ Just (fact "source:" pp_source)
    873 896 
                      , Just (fact "recursivity:" pp_recur)
    ... ... @@ -892,8 +915,8 @@ specConstrProgram guts
    892 915 
             -- unfolding: iface files record no spans for local binders
    893 916 
             pp_source = case (mb_parent, isGoodSrcSpan (nameSrcSpan loc_name)) of
    894 917 
               (Nothing, True)  -> pp_loc
    895 
    -          (Just p,  True)  -> quotes (ppr p) <+> text "at" <+> pp_loc
    896 
    -          (Just p,  False) -> quotes (ppr p) <> comma <+> pp_no_loc
    918 
    +          (Just p,  True)  -> quotes (pp_name p) <+> text "at" <+> pp_loc
    919 
    +          (Just p,  False) -> quotes (pp_name p) <> comma <+> pp_no_loc
    897 920 
               (Nothing, False) -> pp_no_loc
    898 921 
               where
    899 922 
                 loc_name  = rebox_loc_name fn mb_parent
    ... ... @@ -906,7 +929,7 @@ specConstrProgram guts
    906 929 
               ReboxNonRec False   -> text "non-recursive"
    907 930 
               ReboxMutualRec sibs
    908 931 
                 -> text "mutually recursive with"
    909 
    -               <+> pprWithCommas (quotes . ppr) named <> pp_rest
    932 
    +               <+> pprWithCommas (quotes . pp_name) named <> pp_rest
    910 933 
                 where
    911 934 
                   (named, rest) = splitAt 3 (sortBy stableNameCmp sibs)
    912 935 
                   pp_rest = case length rest of
    ... ... @@ -916,7 +939,7 @@ specConstrProgram guts
    916 939
    917 940 
             pp_callers = case sortBy stableNameCmp callers of
    918 941 
               [] -> Nothing
    919 
    -          cs -> Just (pprWithCommas (quotes . ppr) cs)
    942 
    +          cs -> Just (pprWithCommas (quotes . pp_name) cs)
    920 943
    921 944 
             pats_label = case pats of
    922 945 
               [_] -> "call pattern:"
    ... ... @@ -926,12 +949,12 @@ specConstrProgram guts
    926 949
    927 950 
             -- "-- reboxes" and "--      as" have equal width, aligning the
    928 951 
             -- payloads; a wrapping signature continues under the type's start
    929 
    -        pp_pat (ReboxedPat shapes cons spec_occ spec_ty)
    930 
    -          = hang (hang (ppr fn) 2 (fsep (map pprPatShape shapes))) 2 $ vcat
    952 
    +        pp_pat (ReboxedPat shapes cons spec_nm spec_ty)
    953 
    +          = hang (hang (pp_name fn) 2 (fsep (map pprPatShape shapes))) 2 $ vcat
    931 954 
                   [ text "-- reboxes" <+>
    932 955 
                     pprWithCommas pp_con (sortBy stableNameCmp cons)
    933 956 
                   , text "--      as" <+>
    934 
    -                quotes (ppr spec_occ <+> dcolon <+> pp_ty spec_ty) ]
    957 
    +                quotes (pp_name spec_nm <+> dcolon <+> pp_ty spec_ty) ]
    935 958
    936 959 
             -- Qualify imported constructors: they identify the package to
    937 960 
             -- follow up with when the function itself has no location.
    ... ... @@ -1614,9 +1637,13 @@ that decision bites, without changing which specialisations are made:
    1614 1637 
       the common inlined-library-loop case — Core Tidy drops them.)  Both
    1615 1638 
       parts of the signature are approximate:
    1616 1639
    1617 
    -  - The name is the spec's occurrence name at creation ($s<fn>); tidying
    1618 
    -    can prefix the parent and suffix a digit (`$sgo` may end up as
    1619 
    -    `go_$sgo1`), so it is a substring of the final name.
    1640 
    +  - The name is the spec binder's at creation, unique included
    1641 
    +    ($s<fn>_sXYZ): it matches -ddump-spec-constr output exactly.  A spec
    1642 
    +    that stays local keeps it through Tidy Core (-ddump-simpl); one
    1643 
    +    floated to top level is renamed by Core Tidy (`$sgo` may end up as
    1644 
    +    `go_$sgo1` with a fresh unique), and STG dumps renumber all uniques
    1645 
    +    — there only the occurrence-name part is a substring of the final
    1646 
    +    name.
    1620 1647
    1621 1648 
       - The type is the spec binder's type at creation.  It normally survives
    1622 1649 
         to the final program — the default pipeline runs no worker/wrapper
    ... ... @@ -1635,14 +1662,18 @@ that decision bites, without changing which specialisations are made:
    1635 1662 
       only freshens a binder on an in-scope clash, which cannot arise
    1636 1663 
       between sibling top-level RHSs.
    1637 1664
    1638 
    -  Warnings that would render identically — same name, type, parent,
    1639 
    -  definition site, recursivity, and patterns — are merged too: the
    1640 
    -  reader could not tell them apart, so printing both is noise.  For
    1641 
    -  located functions the merged warnings are simplifier-made copies of one
    1642 
    -  binding, addressed by a single source-level remedy; span-less loops
    1643 
    -  inlined from other modules can in principle merge across different
    1644 
    -  origins, but sharing name and type they are almost certainly copies of
    1645 
    -  one function.
    1665 
    +  Local binder names in the warning — the function, its parent, the
    1666 
    +  callers, and the spec — display with their uniques, exactly as
    1667 
    +  -ddump-spec-constr prints them, so unique-distinct copies render
    1668 
    +  distinctly and each can be grepped in that dump.  Under
    1669 
    +  -dsuppress-uniques the uniques disappear, and warnings that would then
    1670 
    +  render identically — same name, type, parent, definition site,
    1671 
    +  recursivity, and patterns — are merged: the reader could not tell
    1672 
    +  them apart, so printing both is noise.  For located functions the
    1673 
    +  merged warnings are simplifier-made copies of one binding, addressed
    1674 
    +  by a single source-level remedy; span-less loops inlined from other
    1675 
    +  modules can in principle merge across different origins, but sharing
    1676 
    +  name and type they are almost certainly copies of one function.
    1646 1677
    1647 1678 
     * The warning classifies how the specialised function recurses
    1648 1679 
       ("recursivity"), taken from the binding SpecConstr saw: a Rec group of
    ... ... @@ -2277,7 +2308,7 @@ specialise env recur bind_calls (RI { ri_fn = fn, ri_lam_bndrs = arg_bndrs
    2277 2308 
                   rebox_ws = [ SpecReboxed (idName fn) (idType fn) (sc_top_fn env)
    2278 2309 
                                            recur
    2279 2310 
                                            [ReboxedPat (patShapes p) (cp_rebox p)
    2280 
    -                                                   (getOccName spec_id) (idType spec_id)]
    2311 
    +                                                   (idName spec_id) (idType spec_id)]
    2281 2312 
                                            (cp_callers p)
    2282 2313 
                              | (p, OS { os_id = spec_id }) <- new_pats `zip` new_specs
    2283 2314 
                              , not (null (cp_rebox p)) ]
    ... ... @@ -2845,9 +2876,9 @@ instance Outputable CallPat where
    2845 2876
    2846 2877 
     -- | One call pattern as displayed by the reboxing warning: the shapes of
    2847 2878 
     -- the pattern's arguments, the reboxed constructors among them, and the
    2848 
    --- occurrence name and type of the specialisation made for the pattern.
    2879 
    +-- name and type of the specialisation made for the pattern.
    2849 2880 
     -- See Note [Reboxing warning]
    2850 
    -data ReboxedPat = ReboxedPat [PatShape] [Name] OccName Type
    2881 
    +data ReboxedPat = ReboxedPat [PatShape] [Name] Name Type
    2851 2882
    2852 2883 
     -- | The constructor skeleton of one call-pattern argument, as displayed
    2853 2884 
     -- by the reboxing warning

  • docs/users_guide/using-warnings.rst
    ... ... @@ -551,26 +551,32 @@ of ``-W(no-)*``.
    551 551 
         the name and type the specialisation was created with::
    552 552
    553 553 
             call patterns:
    554 
    -          go (_ : _) (Bin _ _ _)
    554 
    +          go_s2Xy (_ : _) (Bin _ _ _)
    555 555 
                 -- reboxes ‘Bin’
    556 
    -            --      as ‘$sgo :: Int -> Int -> Map Int Bool -> Bool’
    556 
    +            --      as ‘$sgo_s3k1 :: Int -> Int -> Map Int Bool -> Bool’
    557 557
    558 558 
         One warning is emitted per
    559 
    -    specialised function, and warnings that would read identically are
    560 
    -    merged into one. Specialisations on nullary constructors are not
    561 
    -    reported, since "reboxing" a nullary constructor simply references
    562 
    -    its shared static closure.
    563 
    -
    564 
    -    The ``as`` signature is a guide for finding the specialisation in a
    565 
    -    Core dump (:ghc-flag:`-ddump-simpl`), for example to judge how much
    566 
    -    reboxing survives optimisation. Later passes may rename the binder —
    567 
    -    typically to ``<parent>_$s<function>``, possibly with a digit appended
    568 
    -    — so search for the shown name as a substring; the type normally
    569 
    -    survives unchanged. A specialisation can also be inlined, or merged
    570 
    -    with another one, and then appears in no dump. The calls rewritten to
    571 
    -    use specialisations can be traced with
    572 
    -    :ghc-flag:`-ddump-rule-firings`; the rewrite rules are named
    573 
    -    ``SC:<function><n>``.
    559 
    +    specialised function. Under :ghc-flag:`-dsuppress-uniques`, warnings
    560 
    +    that would read identically are merged into one. Specialisations on
    561 
    +    nullary constructors are not reported, since "reboxing" a nullary
    562 
    +    constructor simply references its shared static closure.
    563 
    +
    564 
    +    Compiler-generated bindings are shown with their unique suffix (as
    565 
    +    in ``$sgo_s2Xy``), matching the Core dump of the SpecConstr pass
    566 
    +    (:ghc-flag:`-ddump-spec-constr`) of the same compilation, where the
    567 
    +    specialisation can therefore be located verbatim — for example to
    568 
    +    judge how much reboxing survives optimisation. In final Core
    569 
    +    (:ghc-flag:`-ddump-simpl`) a specialisation that remained local to
    570 
    +    its enclosing binding still carries the same name, unique included;
    571 
    +    one floated to the top level is renamed — typically to
    572 
    +    ``<parent>_$s<function>``, possibly with a digit appended and a
    573 
    +    fresh unique. In STG dumps (:ghc-flag:`-ddump-stg-final`) all
    574 
    +    uniques are renumbered. Where the exact name fails, search for the
    575 
    +    shown name without its unique as a substring; the type normally
    576 
    +    survives unchanged. A specialisation can also be inlined, or merged with
    577 
    +    another one, and then appears in no dump. The calls rewritten to use
    578 
    +    specialisations can be traced with :ghc-flag:`-ddump-rule-firings`;
    579 
    +    the rewrite rules are named ``SC:<function><n>``.
    574 580
    575 581 
         A ``source:`` reading ``inlined from another module (no source
    576 582 
         location)`` concerns a function that reached the module being compiled

  • testsuite/tests/simplCore/should_compile/all.T
    ... ... @@ -613,16 +613,16 @@ test('T27589', [grep_errmsg(r'wombat')], compile, ['-O -ddump-simpl -dno-typeabl
    613 613 
     test('T27590', [grep_errmsg(r'wombat')], compile, ['-O -ddump-simpl -dno-typeable-binds -dsuppress-uniques'])
    614 614
    615 615 
     # Tests for -Wspec-constr-reboxing (#27628)
    616 
    -test('T27628', normal, compile, ['-O2 -Wspec-constr-reboxing'])
    617 
    -test('T27628b', normal, compile, ['-O2 -Wspec-constr-reboxing'])
    618 
    -test('T27628c', normal, compile, ['-O2 -Wspec-constr-reboxing'])
    619 
    -test('T27628d', normal, compile, ['-O2 -Wspec-constr-reboxing'])
    620 
    -test('T27628e', normal, compile, ['-O2 -Wspec-constr-reboxing'])
    621 
    -test('T27628f', normal, compile, ['-O2 -Wspec-constr-reboxing'])
    616 
    +test('T27628', normal, compile, ['-O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    617 
    +test('T27628b', normal, compile, ['-O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    618 
    +test('T27628c', normal, compile, ['-O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    619 
    +test('T27628d', normal, compile, ['-O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    620 
    +test('T27628e', normal, compile, ['-O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    621 
    +test('T27628f', normal, compile, ['-O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    622 622 
     test('T27628g', normal, compile, ['-O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    623 623 
     test('T27628h', [extra_files(['T27628h_M.hs'])], multimod_compile, ['T27628h', '-v0 -O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    624 624 
     test('T27628i', [extra_files(['T27628i_M.hs'])], multimod_compile, ['T27628i', '-v0 -O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    625 
    -test('T27628j', normal, compile, ['-O2 -Wspec-constr-reboxing'])
    626 
    -test('T27628k', normal, compile, ['-O2 -Wspec-constr-reboxing'])
    625 
    +test('T27628j', normal, compile, ['-O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    626 
    +test('T27628k', normal, compile, ['-O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    627 627 
     test('T27628l', normal, compile, ['-O2 -Wspec-constr-reboxing -dsuppress-uniques'])
    628 628 
     test('T27628m', [extra_files(['T27628h_M.hs'])], multimod_compile, ['T27628m', '-v0 -O2 -Wspec-constr-reboxing -dsuppress-uniques'])

    • —
    View it on GitLab.
    You're receiving this email because of your account on gitlab.haskell.org. Manage all notifications · Help Notification message regarding https://gitlab.haskell.org/ghc/ghc/-/compare/ba495af13cadbef3c88e38188c8aa85956e002fd...7b0558463c4befb36a076784abe63c8c276a8890 at 1787643104