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 MarkupRich 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)

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 (2kB - 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 (58kB - application/octet-stream)
file #18028:  trampoline_r-arm.patch added by shmuma (645B - application/octet-stream)
file #18024:  ffcall-aclocal.patch added by shmuma (1kB - application/octet-stream)
file #18013:  ffcall-armel-cvs.patch added by shmuma (59kB - application/octet-stream)
file #17976:  ffcall-armel.patch added by shmuma (50kB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -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-cleanup