#7620 closed bug (fixed)

Via-C unregisterised build fails for integer-simple

Reported by: singpolyma Owned by:
Priority: high Milestone: 7.8.1
Component: Compiler Version: 7.7
Keywords: unregistered via-c integer-simple Cc: 0@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Building GHC failed Difficulty: Unknown
Test Case: Blocked By:
Blocking: #7623 Related Tickets:

Description

The cross-compiling toolchain I used can be had from https://developer.blackberry.com/develop/platform_choice/ndk.html

./configure --target=arm-unknown-nto-qnx8.0.0eabi --enable-unregistered

I will attach my build.mk to this ticket, but I did *not* set it to use LLVM for this attempt.

I have also attached two make.log files. One that succeeds up to #7490, and another quite short one that fails very fast with:

when making flags consistent: Warning:
    Compiler unregisterised, so compiling via C
/tmp/ghc25531_0/ghc25531_0.hc: In function 'c2pA_entry':

/tmp/ghc25531_0/ghc25531_0.hc:3691:1:
     warning: this decimal constant is unsigned only in ISO C90 [enabled by default]

/tmp/ghc25531_0/ghc25531_0.hc:3691:17:
     error: expected ')' before numeric constant

Can current integer-simple not be compiled via-C for unregistered?

Attachments (7)

make.log.xz (46.4 KB) - added by singpolyma 15 months ago.
Log of make up until expected failure
make2.log (1.8 KB) - added by singpolyma 15 months ago.
Failure on running make again
config.log.xz (13.9 KB) - added by singpolyma 15 months ago.
Output of the configure command
build.mk (4.8 KB) - added by singpolyma 15 months ago.
config.mk (30.5 KB) - added by singpolyma 15 months ago.
A useful output of the configure script
0001-De-Optimized-RegOff.patch (1.5 KB) - added by psycotica0 15 months ago.
Patch To Fix Code Gen Issue
0001-hopefully-fix-7620.patch (1.0 KB) - added by simonmar 15 months ago.

Download all attachments as: .zip

Change History (22)

Changed 15 months ago by singpolyma

Log of make up until expected failure

Changed 15 months ago by singpolyma

Failure on running make again

Changed 15 months ago by singpolyma

Output of the configure command

Changed 15 months ago by singpolyma

Changed 15 months ago by singpolyma

A useful output of the configure script

comment:1 Changed 15 months ago by dterei

  • Blocking 7623 added

comment:2 Changed 15 months ago by simonmar

  • Architecture changed from Unknown/Multiple to arm
  • Difficulty set to Unknown
  • Operating System changed from Unknown/Multiple to QNX

comment:3 Changed 15 months ago by psycotica0

Running

"inplace/bin/ghc-stage1" -static  -H64m -O0 -fasm    -package-name integer-simple-0.1.1.0 \
-hide-all-packages -i -ilibraries/integer-simple/. -ilibraries/integer-simple/dist-install/build \
-ilibraries/integer-simple/dist-install/build/autogen -Ilibraries/integer-simple/dist-install/build \
-Ilibraries/integer-simple/dist-install/build/autogen -Ilibraries/integer-simple/.    -optP-include \
-optPlibraries/integer-simple/dist-install/build/autogen/cabal_macros.h -package ghc-prim-0.3.1.0  \
-package-name integer-simple -Wall -XHaskell98 -XCPP -XMagicHash -XBangPatterns -XUnboxedTuples \
-XForeignFunctionInterface -XUnliftedFFITypes -XNoImplicitPrelude -O -fasm  -no-user-package-db \
-rtsopts      -odir libraries/integer-simple/dist-install/build -hidir libraries/integer-simple/dist-install/build \
-stubdir libraries/integer-simple/dist-install/build -hisuf hi -osuf  o -hcsuf hc \
-c libraries/integer-simple/./GHC/Integer/Type.hs \
-o libraries/integer-simple/dist-install/build/GHC/Integer/Type.o

Which is what make runs, gives me the same error.
Adding -keep-hc-file allows one to inspect the generated C-Code.

The problem line looks like:

_s1ny = (_s1nj--2147483648) + (_s1nk + _s1ns);

Which is not valid.

A few of the lines around use 0x80000000U, which happens to be the same binary value on 32-bit systems.
It also corresponds to "half_bound_up" in the Type.hs that generated this bad C.

So, I don't know if this issue is that the code generator should spit out (_s1nj-0x80000000U) like it does in other places, or if the issue is that it should spit out (_s1nj-(-2147483648)), but I don't know enough about the Code Generator to find it myself right now.

comment:4 Changed 15 months ago by psycotica0

  • Operating System changed from QNX to Unknown/Multiple
  • Summary changed from Via-C unregistered QNX ARM build fails to Via-C unregisterised ARM build fails

Oh, also, this was just on arm-linux-gnueabi unregisterised, so QNX is not to blame, it just doesn't generate valid C right now.

comment:5 Changed 15 months ago by psycotica0

  • Cc 0@… added

comment:6 Changed 15 months ago by psycotica0

Ok, so, as I expected, I just confirmed that on my computer, even without ARM, just with unregisterised and my local i386, it fails with the same error.

The code generator just spits out bad C.

comment:7 Changed 15 months ago by singpolyma

  • Architecture changed from arm to Unknown/Multiple
  • Keywords qnx removed

comment:8 Changed 15 months ago by singpolyma

  • Summary changed from Via-C unregisterised ARM build fails to Via-C unregisterised build fails for integer-simple

Changed 15 months ago by psycotica0

Patch To Fix Code Gen Issue

comment:9 Changed 15 months ago by singpolyma

  • Status changed from new to patch

comment:10 Changed 15 months ago by igloo

The patch looks right to me, but I think there might be a larger problem here.

halfBoundUp and fullBound are of type Digit, which is a synonym for Word#, so why do we end up representing them with Int?

comment:11 Changed 15 months ago by simonmar

The patch is a bit heavy-handed - the special case for negative offsets is not an optimisation, it's there to make the generated C easier to read, and it's pretty important (IMHO).

It would be good to look into @igloo's concerns too while we're here. I'm also wondering why my unregisterised builds using integer-simple didn't run into this...

Changed 15 months ago by simonmar

comment:12 Changed 15 months ago by simonmar

  • Milestone set to 7.8.1
  • Priority changed from normal to high

I attached another patch, if someone experiencing the problem could verify that it works, that would be great.

comment:13 Changed 15 months ago by psycotica0

This patch seems to work as well as mine.

For the line in question it provides the same output, but I can understand not wanting to have every negative number be represented as _blah+-number.

comment:14 Changed 15 months ago by marlowsd@…

commit b91f3d260f3ccb57e8fcd61f943c6fadf26391c8

Author: Simon Marlow <marlowsd@gmail.com>
Date:   Mon Jan 28 10:04:34 2013 +0000

    hopefully fix #7620

 compiler/cmm/PprC.hs |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

comment:15 Changed 15 months ago by simonmar

  • Resolution set to fixed
  • Status changed from patch to closed
Note: See TracTickets for help on using tickets.