UNREG: fix CmmRegOff large offset handling on W64 platforms
ClosedPublic

Authored by trofi on Jun 16 2018, 6:17 AM.

Details

Summary

Gabor noticed C warning when building unregisterised
64-bit compiler on GHC.Integer.Types (from integer-simple).

Minimised example with a warning:

haskell
{-# LANGUAGE MagicHash #-}
{-# LANGUAGE NoImplicitPrelude #-}
{-# OPTIONS_GHC -Wall #-}

module M (bug) where

import GHC.Prim (Word#, minusWord#, ltWord#)
import GHC.Types (isTrue#)

-- assume Word = Word64
bug :: Word# -> Word#
bug x = if isTrue# (x `ltWord#` 0x8000000000000000##) then 0##
        else x `minusWord#` 0x8000000000000000##
$ LANG=C x86_64-UNREG-linux-gnu-ghc -O1 -c M.hs -fforce-recomp
/tmp/ghc30219_0/ghc_1.hc: In function 'M_bug_entry':

/tmp/ghc30219_0/ghc_1.hc:20:14: error:
     warning: integer constant is so large that it is unsigned

It's caused by limited handling of integer literals in CmmRegOff.
This change switches to use standard integer literal pretty-printer.

C code before the change:

c
FN_(M_bug_entry) {
W_ _sAg;
_cAr:
_sAg = *Sp;
switch ((W_)(_sAg < 0x8000000000000000UL)) {
case 0x1UL: goto _cAq;
default: goto _cAp;
}
_cAp:
R1.w = _sAg+-9223372036854775808;
// ...

C code after the change:

c
FN_(M_bug_entry) {
W_ _sAg;
_cAr:
_sAg = *Sp;
switch ((W_)(_sAg < 0x8000000000000000UL)) {
case 0x1UL: goto _cAq;
default: goto _cAp;
}
_cAp:
R1.w = _sAg+(-0x8000000000000000UL);

URL: https://mail.haskell.org/pipermail/ghc-devs/2018-June/015875.html
Reported-by: Gabor Greif
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>

Test Plan

test generated code on unregisterised mips64 and amd64

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.
trofi created this revision.Jun 16 2018, 6:17 AM
ggreif accepted this revision.Jun 16 2018, 8:29 AM

Cool, thanks!

This revision is now accepted and ready to land.Jun 16 2018, 8:29 AM
bgamari accepted this revision.Jun 16 2018, 10:32 AM

Looks reasonable to me.

This revision was automatically updated to reflect the committed changes.