buglibffcall - foreign function call libraries - Bugs: bug #22492, porting to ARM EABI

 
 

bug #22492: porting to ARM EABI

Submitted by:  Sam Steingold <sds>
Submitted on:  Wed 05 Mar 2008 02:54:25 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: portStatus: Fixed
Privacy: PublicAssigned to: Sam Steingold <sds>
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sat 11 Feb 2017 03:05:41 PM UTC, comment #32:

Additional fixes for ARM were delivered:

http://git.savannah.gnu.org/gitweb/?p=libffcall.git;a=commit;h=acfc07cd7efc53d3df363113b5c4dd68596850c8

http://git.savannah.gnu.org/gitweb/?p=libffcall.git;a=commit;h=10ae54edacddb63e8fdeb05bbc0e91fea2540dfb

http://git.savannah.gnu.org/gitweb/?p=libffcall.git;a=commit;h=121b65020df961416de617cf2ce849d72a54155e

http://git.savannah.gnu.org/gitweb/?p=libffcall.git;a=commit;h=1db81bebac03e5ea0bfe490ecf16d06f336fea18

Bruno Haible <haible>
Project Administrator
Wed 29 Apr 2009 08:09:18 PM UTC, comment #31:

Thanks! It works now, great :).

Max Lapan <shmuma>
Wed 29 Apr 2009 07:58:15 PM UTC, comment #30:

committed

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Wed 29 Apr 2009 07:47:48 PM UTC, comment #29:

This comment is lying us. Result of sizeof(T) (which is struct {char a[3]; }) is 3 (char is not aligned in armel). In PCS for ARM (link to pdf is below), page 19: "A Composite Type not larger than 4 bytes is returned in r0." So, this struct also must be returned in registers. Thus, <= is absolutely necessary to handle this case.

Max Lapan <shmuma>
Wed 29 Apr 2009 07:21:19 PM UTC, comment #28:

I committed the Makefile.devel patch.

the C patch uses "<= sizeof" which is quite unusual:
all other comparisons with sizeof in that directory are for equality.
why are you doing this?

the comment says that the struct size is divisible by 4
and sizeof(int) is 4, so it would seem that <=sizeof(int) is the same as ==sizeof(int).

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Wed 29 Apr 2009 07:02:54 PM UTC, comment #27:

Avcall from CVS still fails at test T f(T,char,T):({"the"},' ',{"fox"})->{"box"}. Please, apply last patch: http://savannah.gnu.org/bugs/download.php?file_id=18045

Max Lapan <shmuma>
Tue 28 Apr 2009 05:28:18 PM UTC, comment #26:

Now it compiles correctly and passes 99% of extracheck :). 99% because I foregot about one small case in avcall. Patch is in attachment, please include.

(file #18045)

Max Lapan <shmuma>
Tue 28 Apr 2009 04:05:05 PM UTC, comment #25:

I committed a patch which uses AC_EGREP_CPP, similar to other platforms.
I also regenerated the configures.
please do a fresh cvs checkout (or "cvs up -C") and make sure everything works.

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Tue 28 Apr 2009 03:19:20 PM UTC, comment #24:

Attached patch for armel detection via _ARMEL_ macro.

(file #18040)

Max Lapan <shmuma>
Tue 28 Apr 2009 06:27:57 AM UTC, comment #23:

No, I think that this check must be done in distro-independed way. I'll do this today and send patch.

Max Lapan <shmuma>
Tue 28 Apr 2009 03:06:04 AM UTC, comment #22:

I see...
so, is armel debian-specific?
what is it?
do you think it is reasonable to limit armel support to debian?

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Tue 28 Apr 2009 02:39:11 AM UTC, comment #21:

No, uname and arch gives us only "arm" which is not enough. Before yesterday, I thought that dpkg-architecture is the only way to detect armel. Now, we can check for _ARMEL_ macro in C.

Max Lapan <shmuma>
Mon 27 Apr 2009 07:15:38 PM UTC, comment #20:

I see. sorry.
now, you are using a debian-specific command to detect armel.
are you sure this is the right way?
e.g., do arch or uname give enough information?

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Mon 27 Apr 2009 07:03:21 PM UTC, comment #19:

But we still need to set host_cpu_abi=armel to vacall and avcall use correct -armel.s files.

Max Lapan <shmuma>
Mon 27 Apr 2009 06:55:20 PM UTC, comment #18:

the only changes to configures stem from
callback/trampoline_r/configure.in and
trampoline/configure.in.
since you are not defining _armel_ and using _ARMEL_ from gcc instead, the patch to general.m4 is not necessary.

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Mon 27 Apr 2009 05:50:35 PM UTC, comment #17:

You probably missed something:

aclocal.m4 is 3 days old: http://cvs.savannah.gnu.org/viewvc/ffcall/aclocal.m4?root=libffcall&view=log

configure in vacall (as well as in other subdirs) doesn't have armel detection code: http://cvs.savannah.gnu.org/viewvc/ffcall/vacall/configure?revision=1.55&root=libffcall&view=markup

Max Lapan <shmuma>
Mon 27 Apr 2009 05:33:36 PM UTC, comment #16:

I did regenerate the configures.
please do "cvs up -C"

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Mon 27 Apr 2009 05:10:21 PM UTC, comment #15:

configure scripts needs regeneration

Max Lapan <shmuma>
Mon 27 Apr 2009 04:51:18 PM UTC, comment #14:

committed.
please do a fresh CVS checkout and make sure that everything is OK.
thanks.

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Mon 27 Apr 2009 04:24:08 PM UTC, comment #13:

New version of patch. Removed _armel_ and CFLAGS modification. trampoline_r fix in separate patch.

(file #18027, file #18028)

Max Lapan <shmuma>
Mon 27 Apr 2009 03:50:37 PM UTC, comment #12:

> in general, unrelated changes should be in different change sets.

ok. Should I open separate bug for this? :)

Max Lapan <shmuma>
Mon 27 Apr 2009 03:47:56 PM UTC, comment #11:

> http://www.google.com/codesearch?q=_ARMEL++lang:c
> seems to indicate that ARMEL is defined.


Hm, really it does. I'll update patch and remove CFLAGS.

Max Lapan <shmuma>
Mon 27 Apr 2009 03:44:58 PM UTC, comment #10:

> IIUC, the only difference between armel and armeb (aka arm)
> is the endianness.
> maybe the better way is to use a generic BIG_ENDIAN/LITTLE_ENDIAN constant?


As I learn from comparing recent PCS for ARM (http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042b/IHI0042B_aapcs.pdf) and ffcall code, the difference is not only in endiannes. _arm_ and _armel_ differs in:
1. alignment (double values aligned by 8 bytes),
2. floating point handling (armel have no direct access to floating-point registers and do this via generic registers),
3. small structure return convention (armel can return max 4-bytes structs in registers),
4. cache flush method (cache flush code in CVS should work, but actually doesn not).

Max Lapan <shmuma>
Mon 27 Apr 2009 03:30:21 PM UTC, comment #9:

http://www.google.com/codesearch?q=_ARMEL++lang:c
seems to indicate that _ARMEL_ is defined.

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Mon 27 Apr 2009 03:22:35 PM UTC, comment #8:

in general, unrelated changes should be in different change sets.

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Mon 27 Apr 2009 03:15:38 PM UTC, comment #7:

> We need both arm and armel defined.
> GCC gives us only arm on all arm-like platforms which
> is too broad to distinguish armel from others.
> Maybe there are other way or better place
> to set this directives, but code needs them both.


IIUC, the only difference between armel and armeb (aka arm)
is the endianness.
maybe the better way is to use a generic BIG_ENDIAN/LITTLE_ENDIAN constant?

what do other packages do?

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Mon 27 Apr 2009 02:24:19 PM UTC, comment #6:

> 1. the change to aclocal.m4:
> A. this is a generated file.
> the change should go to m4/general.m4,
> the function FFCALL_CANONICAL_HOST_CPU.


Hm, really, dull mistake. Not much familiar with autoconf :). Fix for this attached.

> B. the change sets CFLAGS.
> why?


We need both _arm_ and _armel_ defined. GCC gives us only _arm_ on all arm-like platforms which is too broad to distinguish armel from others. Maybe there are other way or better place to set this directives, but code needs them both.

> 2. the change to callback/trampoline_r/trampoline.c:
> you are modifying generic (non-armel-specific) code.
> why?


In CVS we have obvious bug with trampoline detection on all arms. Look at this (code from CVS head):

#ifdef _arm_
/* function:
* add r12,pc,#8 E28FC008
* ldr pc,[pc] E59FF000
* _data:
* .word <data>
* _function:
* .word <address>
*/
{
((long *) function)[0] = 0xE28FC008;
((long *) function)[1] = 0xE59FF000;
((long *) function)[2] = (long) data;
((long *) function)[3] = (long) address;
}
#define is_tramp(function) \
((long *) function)[0] == 0xE28FC004 && \
((long *) function)[1] == 0xE51FF004
#define tramp_address(function) \
((long *) function)[3]
#define tramp_data(function) \
((long *) function)[2]
#endif

In trampoline/trampoline.c macro is_tramp works correctly.

> 3. you want to include config.h in tests.c files.
> why?


In *call tests there are checks for HAVE_LONG_LONG_INT which defined during configure in confg.h. So, we have to include this file to test long long functionality.

(file #18024)

Max Lapan <shmuma>
Mon 27 Apr 2009 01:42:09 PM UTC, comment #5:

thanks, now it applies cleanly.
however, there are a few problems:

1. the change to aclocal.m4:
A. this is a generated file.
the change should go to m4/general.m4,
the function FFCALL_CANONICAL_HOST_CPU.
B. the change sets CFLAGS.
why?

2. the change to callback/trampoline_r/trampoline.c:
you are modifying generic (non-armel-specific) code.
why?

3. you want to include config.h in tests.c files.
why?

Thanks!

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Sat 25 Apr 2009 06:53:24 PM UTC, comment #4:

Sorry, my fault. Took source tree with debian patches. Correct patch is attached.

(file #18013)

Max Lapan <shmuma>
Fri 24 Apr 2009 02:52:49 PM UTC, comment #3:

alas, ffcall-armel.patch does not apply cleanly to the current cvs head. could you please update it?
thanks

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Mon 20 Apr 2009 04:48:24 PM UTC, comment #2:

Hello,

in attachment is my patch against 1.10 release for arm eabi. It successfully passes all ffcall tests both in scratchbox and real device. Git with individual commits is here: http://www.github.com/Shmuma/ffcall/

(file #17976)

Max Lapan <shmuma>
Fri 26 Sep 2008 08:25:14 PM UTC, comment #1:

this might be obsolete now that an arm patch has been merged in.
please try ffcall cvs head

Sam Steingold <sds>
Project AdministratorIn charge of this item.
Wed 05 Mar 2008 02:54:25 PM UTC, original submission:

http://permalink.gmane.org/gmane.lisp.clisp.devel/17675

I had a go at porting clisp, particularly ffcall,
to ARM's new EABI, so
that I could run it on my Nokia N810.
I got vacall and avcall working,
but trampoline defeated me.
Unfortunately I don't have time to finish
this off, so I'm just dumping my results out to the world:

http://mat.exon.name/logs/clisp
http://mat.exon.name/logs/ffcall-arm.diff

If anyone wants to pick up where I left off,
those notes might help.
I'm afraid I can't take suggestions for what I should try next,
since I'm taking my lisp-less Nokia on the road for six months
and won't have access to a development machine.

Sam Steingold <sds>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #18045:  ffcall-avcall-sstructs.patch added by shmuma (2KiB - application/octet-stream)
file #18040:  ffcall-armel-detect.patch added by shmuma (597B - application/octet-stream)
file #18027:  ffcall-armel-cvs-clean.patch added by shmuma (58KiB - application/octet-stream)
file #18028:  trampoline_r-arm.patch added by shmuma (645B - application/octet-stream)
file #18024:  ffcall-aclocal.patch added by shmuma (1KiB - application/octet-stream)
file #18013:  ffcall-armel-cvs.patch added by shmuma (59KiB - application/octet-stream)
file #17976:  ffcall-armel.patch added by shmuma (50KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by haible (Posted a comment)
  • -unavailable- added by shmuma (Updated the item)
  • -unavailable- added by sds (the original patch submitter)
  • -unavailable- added by sds (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 12 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 28 Apr 2009 05:41:00 PM UTCsdsItem GroupNone=>port
    Tue 28 Apr 2009 05:28:18 PM UTCshmumaAttached File-=>Added ffcall-avcall-sstructs.patch, #18045
    Tue 28 Apr 2009 03:19:20 PM UTCshmumaAttached File-=>Added ffcall-armel-detect.patch, #18040
    Mon 27 Apr 2009 04:51:18 PM UTCsdsStatusNone=>Fixed
      Assigned tohaible=>sds
      Open/ClosedOpen=>Closed
    Mon 27 Apr 2009 04:24:08 PM UTCshmumaAttached File-=>Added ffcall-armel-cvs-clean.patch, #18027
      Attached File-=>Added trampoline_r-arm.patch, #18028
    Mon 27 Apr 2009 02:24:19 PM UTCshmumaAttached File-=>Added ffcall-aclocal.patch, #18024
    Sat 25 Apr 2009 06:53:24 PM UTCshmumaAttached File-=>Added ffcall-armel-cvs.patch, #18013
    Mon 20 Apr 2009 04:48:24 PM UTCshmumaAttached File-=>Added ffcall-armel.patch, #17976
    Fri 26 Sep 2008 08:25:14 PM UTCsdsCarbon-Copy-=>Added -unavailable-

    Back to the top


    Powered by Savane 3.1-cleanup1