Ticket #308 (closed patch: fixed)

Opened 6 years ago

Last modified 6 years ago

[PATCH] [imcc] Allow for 128-bit long doubles

Reported by: doughera Owned by: rurban
Priority: minor Milestone:
Component: imcc Version:
Severity: low Keywords:
Cc: Language:
Patch status: new Platform:

Description

This patch allows imcc to handle FLOATVALS with size == 16. (It already handled sizes of 8 and 12.)

I've also changed the fall-through case from a misspelled useless run-time warning to an error. If I understood why any of this was needed in the first place, I might have an informed opinion about whether or not any error at all should be issued. Meanwhile, this is better than the current state of affairs.

With this patch, Configure.pl --floatval='long double' should now build on 64-bit systems that have 128-bit long doubles. There are still some test failures with this change, but they look to me like errors in the test suite. In any case, there are fewer errors than without this change.

Attachments

imcc-floatsize.patch Download (1.2 KB) - added by doughera 6 years ago.
tt308-long-double16.patch Download (34.2 KB) - added by rurban 6 years ago.
rel-2
tt308-more-pf_items.patch Download (19.9 KB) - added by rurban 6 years ago.

Change History

Changed 6 years ago by doughera

Changed 6 years ago by rurban

  • owner set to rurban
  • status changed from new to assigned
  • patch set to new

Thanks. Is this solaris/x86 with sunpro cc and -m64? I get hugefloatvalsize=16 there and I can reproduce it.

We also need to add pbc converters for this new floattype=2 to be able to read such pbc's. This needs a little bit of more work. See my tt308-imcc-floatsize.patch

t/compilers/imcc/imcpasm/opt1 (Wstat: 256 Tests: 78 Failed: 1)

Failed test: 75

t/op/jitn (Wstat: 256 Tests: 14 Failed: 1)

Failed test: 14

t/op/number (Wstat: 256 Tests: 56 Failed: 1)

Failed test: 40

Changed 6 years ago by doughera

I actually tested with Solaris/SPARC. The test failures you noted are arguably errors in the tests.

I can think of 5 different floatval types: 8-byte (big endian, little endian), 16-byte (big endian, little endian) and 12-byte (little endian only-- I think that's an x86-only invention). Some sort of symbolic constants rather than magic numbers are needed in src/packfile.c.

Reading and writing these different formats where they are native ought to generally work now -- the packfile stuff is deliberately written in terms of native operations with appropriate sizeof() operators to try to make this all work well. Reading non-native pbcs is likely to need a *lot* of work. The current reading routines are geared towards stepping through the file in steps of size sizeof(opcode_t). If that doesn't match the sizeof(opcode_t) with which the file was written, trouble will surely ensue. The non-native reader ought to acknowledge that fundamental difference up front and change the way it reads, rather than trying to paper over the differences here and there with various ad-hoc fix-ups.

I think reading non-native pbcs will require a lot of work, but probably isn't a high priority for the project now, since the workaround (distribute the pir file instead) is easy.

Changed 6 years ago by rurban

Solaris/SPARC and big-endian long double, good. There we also need native pbc's for the native tests. Can you try tools/dev/mk_native_pbc and send us the generated integer and number pbc?

I'll add little more support for that BE type as well and apply it then, since it breaks nothing and adds basic support for it.

5 floattypes, that explodes now the complexity of possible floatval converters to (5*4/2 = 10), which will lead to a redesign of the transformers, probably by passing numsize args to a more general converter. Or rewrite it in one of our high-level languages (nqp => pir => o).

It's very low priority, indeed, but on the nice to have list. A very good marketing argument. "Full cross-platform portability with one binary format only!"

I saw also that MIPS 64 BE has a different format for NaN. Wonder if that deserves a seperate floattype=6 also (6*5/2=15 transformers), or if we should not just use add osname to the header for such quirks. I did that in the perl5 bytecode header. There will be more such specialities I assume.

Changed 6 years ago by doughera

Aargh. I just tried tools/mk_native_pbc. It doesn't work with the old Bourne shell I have installed as /bin/sh. Fair enough. It's an old shell. Fixing that, it calls the first 'perl' in my path, not the one used to build parrot. That matters. That perl is too old to build parrot.

Fixing it to call the correct perl, it then cleans out my build directory! An hour's worth of work gone! Aargh! Oh, well; I let it run to completion. Turns out it called Configure.pl with the wrong arguments. None of the arguments I used to build parrot originally were passed along. It also makes the erroneous assumption that it can't build 'long double' unless my 'perl' was compiled with 'long double', so the whole hour-and-a-half long exercise has been futile anyway, since the script cleaned out the correct configuration and built the wrong one.

I don't have any more time to spend on this.

Changed 6 years ago by rurban

rel-2

Changed 6 years ago by rurban

  • status changed from assigned to closed
  • resolution set to fixed

Using tools/mk_native_pbc is indeed a mess. I just kept the bourne shell, added some sanity checks and cfg detection, but did not yet convert it to perl.

Saving away the old config args really is not on our feature list, you have to store them somewhere else, sorry.

tools/mk_native_pbc --noconf skips the configuration if you need special args

I fixed now all the tests, fixed a tiny problem when running tools/mk_native_pbc with 32-bit perl and 64-bit parrot, and all those tests pass. Few unrelated others still remain.

I use those configs for my tests, from my testvm script.

#sol10
vm_name[$n]=sol10
vm_dir[$n]=/usr/src/perl/parrot
vm_conf[$n]="--ccflags='-m64 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_TS_ERRNO' --ldflags='-m64' --linkflags='-m64'"
vm_make[$n]=/opt/SunStudioExpress/bin/dmake
let n+=1

vm_name[$n]=opensolaris
vm_dir[$n]=/usr/src/perl/parrot
vm_conf[$n]="--ccflags='-m64 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_TS_ERRNO' --ldflags='-m64' --linkflags='-m64' --floatval='long double'"
vm_make[$n]=/opt/SunStudioExpress/bin/dmake
let n+=1

vm_name[$n]=opensolaris
vm_dir[$n]=/usr/src/perl/parrot
vm_conf[$n]=""
vm_make[$n]=/opt/SunStudioExpress/bin/dmake
let n+=1

vm_name[$n]=solgcc
vm_dir[$n]=/usr/src/perl/parrot
vm_conf[$n]="--cc=gcc --ccflags='' --link=ld"
vm_make[$n]=gmake
let n+=1

vm_name[$n]=solgcc
vm_dir[$n]=/usr/src/perl/parrot
vm_conf[$n]="--cc=gcc --ccflags='' --link=ld --floatval='long double'"
vm_make[$n]=gmake
let n+=1 

}}

I'll check this in as it is much better than before. 
Thanks. We don't really need these native_pbc's yet.

Missing are just the 16->8,12 byte pbc converters and testing the new endianize transformers.

Applied as r36689 and r36693.
https://trac.parrot.org/parrot/changeset/36689/

Changed 6 years ago by doughera

  • status changed from closed to reopened
  • resolution fixed deleted

I'm reopening this because it's broken again. Configuring with 'long double' on SPARC yields the following error message:

"src/packfile/pf_items.c", line 1286: undefined symbol: cvt_num8_num16_le
"src/packfile/pf_items.c", line 1286: warning: improper pointer/integer combination: op "="
"src/packfile/pf_items.c", line 1289: undefined symbol: cvt_num12_num16_le
"src/packfile/pf_items.c", line 1289: warning: improper pointer/integer combination: op "="

Changed 6 years ago by rurban

  • status changed from reopened to new

Thanks, I indeed forgot the 64bit BE transformers.

Attached is my current state of affairs, but I'm still in the middle of adding the missing transformer implementations.

And I had no time yet to fix the more general 64bit issues, which needs a new ticket.

Changed 6 years ago by rurban

Changed 6 years ago by rurban

  • status changed from new to closed
  • resolution set to fixed

The 64bit big-endian compilation issues should be fixed with r36819.

I also added more float converters, but some are still missing.

Changed 6 years ago by rurban

for reference: added the last missing converters with r37056.

tests and native_pbc updates on missing platforms configs appreciated.

Note: See TracTickets for help on using tickets.