[commit: ghc] master: make iToBase62's inner loop stricter in one of its arguments (ed78951)

[Simon Peyton Jones]

Alp,

Is this related to #15570.  If so, it'd be good to make that connection.

S

|  -----Original Message-----
|  From: ghc-commits <ghc-commits-bounces at haskell.org> On Behalf Of
|  git at git.haskell.org
|  Sent: 02 September 2018 22:16
|  To: ghc-commits at haskell.org
|  Subject: [commit: ghc] master: make iToBase62's inner loop stricter in
|  one of its arguments (ed78951)
|  
|  Repository : ssh://git@git.haskell.org/ghc
|  
|  On branch  : master
|  Link       :
|  https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fghc.haske
|  ll.org%2Ftrac%2Fghc%2Fchangeset%2Fed789516e201e4fad771e5588da47a62e53b42b
|  8%2Fghc&data=02%7C01%7Csimonpj%40microsoft.com%7Cea714d654b04488aada9
|  08d6111955e2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636715197856324
|  232&sdata=3Oo7y3FCIzEZVAWc%2FPl6ZXwji2QM2CWWAI3UfHqxDn0%3D&reserv
|  ed=0
|  
|  >---------------------------------------------------------------
|  
|  commit ed789516e201e4fad771e5588da47a62e53b42b8
|  Author: Alp Mestanogullari <alp at well-typed.com>
|  Date:   Sun Sep 2 22:06:55 2018 +0200
|  
|      make iToBase62's inner loop stricter in one of its arguments
|  
|      Summary:
|      hadrian's support for dynamic ways is currently broken (see
|  hadrian#641 [1]).
|      The stage 1 GHCs that hadrian produces end up producing bad code for
|      the `iToBase62` function after a few optimisation passes.
|  
|      In the case where `quotRem` returns (overflowError, 0),
|      GHC isn't careful enough to realise q is _|_ and happily inlines,
|      distributes and floats code around until we end up trying to access
|      index `minBound :: Int` of an array of 62 chars, as a result of
|  inlining
|      the definition of `quotRem` for Ints, in particular the minBound
|  branch [2].
|  
|      I will separately look into reproducing the bad transformation on a
|  small
|      self-contained example and filling a ticket.
|  
|      [1]:
|  https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
|  om%2Fsnowleopard%2Fhadrian%2Fissues%2F641&data=02%7C01%7Csimonpj%40mi
|  crosoft.com%7Cea714d654b04488aada908d6111955e2%7C72f988bf86f141af91ab2d7c
|  d011db47%7C1%7C0%7C636715197856324232&sdata=a7dM0YbraOY69xKhJcwVuQqBn
|  3n3nTdVuMFNjie6iHA%3D&reserved=0
|      [2]:
|  https://git.haskell.org/ghc.git/blob/HEAD:/libraries/base/GHC/Real.hs#l36
|  6
|  
|      Test Plan: fixes hadrian#641
|  
|      Reviewers: bgamari, tdammers
|  
|      Reviewed By: tdammers
|  
|      Subscribers: tdammers, rwbarton, carter
|  
|      Differential Revision: https://phabricator.haskell.org/D5106
|  
|  
|  >---------------------------------------------------------------
|  
|  ed789516e201e4fad771e5588da47a62e53b42b8
|   compiler/basicTypes/Unique.hs | 2 +-
|   1 file changed, 1 insertion(+), 1 deletion(-)
|  
|  diff --git a/compiler/basicTypes/Unique.hs
|  b/compiler/basicTypes/Unique.hs index 4a709d2..b5c0fce 100644
|  --- a/compiler/basicTypes/Unique.hs
|  +++ b/compiler/basicTypes/Unique.hs
|  @@ -325,7 +325,7 @@ iToBase62 n_
|       go n cs | n < 62
|               = let !c = chooseChar62 n in c : cs
|               | otherwise
|  -            = go q (c : cs) where (q, r) = quotRem n 62
|  +            = go q (c : cs) where (!q, r) = quotRem n 62
|                                     !c = chooseChar62 r
|  
|       chooseChar62 :: Int -> Char
|  
|  _______________________________________________
|  ghc-commits mailing list
|  ghc-commits at haskell.org
|  https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.hask
|  ell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-
|  commits&data=02%7C01%7Csimonpj%40microsoft.com%7Cea714d654b04488aada9
|  08d6111955e2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636715197856324
|  232&sdata=5oKSdyvdcyJlPR%2FtNUNuptlsFuv9jJA%2FVmc7AOid5mc%3D&rese
|  rved=0