t/op/pack.t fails when compiled with Open64 #12900
Comments
From @HugmeirCreated by @Hugmeir$ opencc --version My configure line is just this: ./Configure -des -Dusedevel -DDEBUGGING -Dcc=opencc Besides a handful of extraneous warnings during compilation, everything not ok 1012 Perl Info |
From @nwc10On Sat, Apr 06, 2013 at 04:36:31AM -0700, Brian Fraser wrote:
Which version *is* that? http://www.open64.net/news.html
Also, this isn't the perl -V from the compiler that you were building with:
which makes it harder to work out what is going on. However, I can (eventually) get Open64 to build. I am going to document I first tried to build the current release, 5.0.0, from source from the To do this, it seems 1) It insists on building 32 bit, so I have to install the gcc multilibs $ diff -u osprey-gcc-4.2.0/host-x86_64-redhat-linux/gcc/Makefile~ osprey-gcc-4.2.0/host-x86_64-redhat-linux/gcc/Makefile Inline Patch--- osprey-gcc-4.2.0/host-x86_64-redhat-linux/gcc/Makefile~ 2013-04-25 20:15:50.000000000 +0200
+++ osprey-gcc-4.2.0/host-x86_64-redhat-linux/gcc/Makefile 2013-04-25 20:28:24.000000000 +0200
@@ -848,7 +848,7 @@
#LIBGSPIN = ../../../libspin/libgspin42.a
#This is a symbol link, create by the CONFIGURE script
#LIBGSPIN = ../../../libspin/libgspin42.a
-LIBGSPIN = ../../osprey/targdir/libspin_4_2_0/libgspin42.a
+LIBGSPIN = ../../../osprey/targdir/libspin_4_2_0/libgspin42.a
# Build and host support libraries.
LIBIBERTY = ../libiberty/libiberty.a
On some Ubuntu distributions, you need to change the symbol link http://www.open64.net/?id=254 I do this. It's a good job the power didn't go out in the middle, as I'd also like to express my utter *contempt* for anything which feels that With this, I can replicate your problem. It seems to be a code generation bug. With the optimisation level you gave: $ ./perl -le 'print unpack "%33i", pack "i", -2147483648 One would expect this: If I turn the optimiser down, it goes away. For the example above, it's this bit of pp_pack.c: case 'i' | TYPE_IS_SHRIEKING: Sticking gdb on there to look at aint produces garbage: (gdb) However, as it's compiled with -O2 as well as -g, it's not clear whether this So I stuck a printf() in there, and the bug goes away. Interesting. Question - is this because printf() is varargs, and so it forces that variable After a bit of experimentation, it seemed that adding any *reference* to Inline Patchdiff --git a/pp_pack.c b/pp_pack.c
index eaeec89..33a65b6 100644
--- a/pp_pack.c
+++ b/pp_pack.c
@@ -1820,6 +1820,8 @@ S_unpack_rec(pTHX_ tempsym_t* symptr, const char *s, const
while (len-- > 0) {
int aint;
SHIFT_VAR(utf8, s, strend, aint, datumtype);
+ if (aint)
+ getpid();
DO_BO_UNPACK(aint, i);
if (!checksum)
mPUSHi(aint);
Inline Patchdiff --git a/pp_pack.c b/pp_pack.c
index eaeec89..3366c59 100644
--- a/pp_pack.c
+++ b/pp_pack.c
@@ -1820,6 +1820,8 @@ S_unpack_rec(pTHX_ tempsym_t* symptr, const char *s, const
while (len-- > 0) {
int aint;
SHIFT_VAR(utf8, s, strend, aint, datumtype);
+ if (checksum)
+ getpid();
DO_BO_UNPACK(aint, i);
if (!checksum)
mPUSHi(aint);
$ diff -u pp_pack.s.bad pp_pack.s.good | diffstat I don't know x86_64 assembler (and there isn't space in my head for it) $ diff -u pp_pack.s.bad pp_pack.s Inline Patch--- pp_pack.s.bad 2013-04-26 16:49:40.000000000 +0200
+++ pp_pack.s 2013-04-26 16:51:29.000000000 +0200
@@ -9416,7 +9416,8 @@
#<loop> Part of loop body line 1822, head labeled .Lt_68_360194
.loc 1 1831 0
movq -464(%rbp),%rax # [0] cuv
- addq %rsi,%rax # [3]
+ movslq %esi,%rdi # [2]
+ addq %rdi,%rax # [3]
movq %rax,-464(%rbp) # [4] cuv
jmp .Lt_68_425730 # [4]
.LBB426_S_unpack_rec:
So, yes, I think that this is a compiler bug. In theory we could try to In practise, I don't think that we should bother 1) It's not obvious how to to do it cleanly There aren't many commits since 2011, and if they aren't careful they will 3) That code in pp_pack.c is a lot more complex than it could be. In SHIFT_VAR() is from Ton Hospel's "encoding neutral unpack" work. It either DO_BO_UNPACK() is from Marcus Holland Moritz's byte-order changes. It flips I think that the byte-order stuff was done first. Right now, the code carefully copies (logical) characters in-order into an I think that a simpler solution would be to migrate that inversion into the However, I can see that that refactoring is going to be a little I guess that this bug is sort of a "wontfix". Nicholas Clark |
|
The RT System itself - Status changed from 'new' to 'open' |
|
@nwc10 - Status changed from 'open' to 'stalled' |
From @HugmeirOn Tue, Apr 30, 2013 at 9:36 AM, Nicholas Clark <nick@ccl4.org> wrote:
http://developer.amd.com/tools-and-sdks/cpu-development/x86-open64-compiler-suite/
Oh, I apologize. IIRC I was building with "blead as of today",
Hm... My experience was also troublesome, but nowhere as annoying as all of
Most of this is beyond me for now, but +1 to the conclusion. Thank you for |
|
The RT System itself - Status changed from 'stalled' to 'open' |
From @nwc10On Tue, Apr 30, 2013 at 03:49:51PM -0300, Brian Fraser wrote:
Oh, interesting. So which one is the fork? :-) That looks like a compiler with more of a future than the open64.net version.
I infer that this is because we were using versions from different alternate It's not obvious how to reduce pp_pack.c down to any sort of test case that I hope that a refactoring of pp_pack.c (with the side effect of making it Nicholas Clark |
From @nwc10On Fri, May 03, 2013 at 12:32:54PM +0100, Nicholas Clark wrote:
The branch has rather diverged from its name, but for me (The branch also removes about 1000 lines of source, and reduces the object Nicholas Clark |
From @nwc10On Thu May 09 11:32:08 2013, nicholas wrote:
This is now merged to blead. Please could you check that blead Nicholas Clark |
From @HugmeirOn Wed, May 22, 2013 at 7:13 AM, Nicholas Clark via RT <
$ ./TEST op/pack.t Nicholas++ |
From @nwc10On Wed May 22 05:14:32 2013, Hugmeir wrote:
Thanks. Marking this ticket "resolved" as we don't (yet) have a Nicholas Clark |
|
@nwc10 - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#117501 (status was 'resolved')
Searchable as RT117501$
The text was updated successfully, but these errors were encountered: