Opened 4 years ago

Closed 4 years ago

#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 Test Case:
Blocked By: Blocking: #7623
Related Tickets: Differential Rev(s):
Wiki Page:

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

Download all attachments as: .zip

Change History (22)

Changed 4 years ago by singpolyma

Attachment: make.log.xz added

Log of make up until expected failure

Changed 4 years ago by singpolyma

Attachment: make2.log added

Failure on running make again

Changed 4 years ago by singpolyma

Attachment: config.log.xz added

Output of the configure command

Changed 4 years ago by singpolyma

Attachment: build.mk added

Changed 4 years ago by singpolyma

Attachment: config.mk added

A useful output of the configure script

comment:1 Changed 4 years ago by dterei

Blocking: 7623 added

comment:2 Changed 4 years ago by simonmar

Architecture: Unknown/Multiplearm
difficulty: Unknown
Operating System: Unknown/MultipleQNX

comment:3 Changed 4 years 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 4 years ago by psycotica0

Operating System: QNXUnknown/Multiple
Summary: Via-C unregistered QNX ARM build failsVia-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 4 years ago by psycotica0

Cc: 0@… added

comment:6 Changed 4 years 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 4 years ago by singpolyma

Architecture: armUnknown/Multiple
Keywords: qnx removed

comment:8 Changed 4 years ago by singpolyma

Summary: Via-C unregisterised ARM build failsVia-C unregisterised build fails for integer-simple

Changed 4 years ago by psycotica0

Patch To Fix Code Gen Issue

comment:9 Changed 4 years ago by singpolyma

Status: newpatch

comment:10 Changed 4 years 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 4 years 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 4 years ago by simonmar

comment:12 Changed 4 years ago by simonmar

Milestone: 7.8.1
Priority: normalhigh

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

comment:13 Changed 4 years 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 4 years 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 4 years ago by simonmar

Resolution: fixed
Status: patchclosed
Note: See TracTickets for help on using tickets.