make iToBase62's inner loop stricter in one of its arguments
ClosedPublic

Authored by alpmestan on Aug 24 2018, 4:35 PM.

Details

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://github.com/snowleopard/hadrian/issues/641
[2]: https://git.haskell.org/ghc.git/blob/HEAD:/libraries/base/GHC/Real.hs#l366

Test Plan

fixes hadrian#641

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
alpmestan created this revision.Aug 24 2018, 4:35 PM

And here's the ticket with the self-contaiend reproducer: https://ghc.haskell.org/trac/ghc/ticket/15570#ticket

tdammers accepted this revision.Aug 28 2018, 2:55 AM
tdammers added a subscriber: tdammers.

I agree that this should be properly solved eventually. Workaround looks fine though.

This revision is now accepted and ready to land.Aug 28 2018, 2:55 AM
This revision was automatically updated to reflect the committed changes.