483259 - Tamarin sources will not compile when building for Android

Status: VERIFIED FIXED

(Tamarin Graveyard :: Build Config, enhancement)

Description

[Paton Lewis]

Attachment: Patch for allowing Tamarin sources to compile for Android

Tamarin sources will not compile when building for Android. However, the attached Mercurial diffs against Tamarin-Central provide the modifications necessary to allow Tamarin sources to build on Android.

Comment 1

[Steven Johnson]

Comment on attachment 367256 [details] [diff] [review]
Patch for allowing Tamarin sources to compile for Android

Assigning Rishit to review -- might be nice to add a VMPI definition for the "aligned" stuff.

Comment 2

[Rishit Shah]

Converting this to VMPI is slightly tricky here.

The syntax currently in use seems to follow Microsoft compiler syntax.

GCC for x86 and PPC supports #pragma pack(push, n) for compatibility with Microsoft compiler.

Realview compilers support #pragma pack(n) and also the __attribute__((aligned(n))) for compatibility with GCC.

GCC for this platform seems to not support the microsoft compiler extension. 

Given the variation in declaration syntax I feel its not worth adding VMPI directives.  Instead we could choose Microsoft vs GCC (given that RVCT supports GNU extension in this case).

Steven, I don't have review privileges so you'll have to change the status.

Comment 3

[Rishit Shah]

On the patch itself I have one comment.

In LIR.h, instead of #ifdef'ing it with the platform name, would it be better to use some compiler feature related flag like COMPILER_SUPPORTS_PRAGMA_PACK or simply a compiler specific flag?

Comment 4

[Lars T Hansen]

Edwin, why does LIR.h use #pragma pack at all?

Comment 5

[Edwin Smith]

Those are overlay structs which were added to eliminate some once-buggy pointer math.  it might be possible to go back to something more portable and not-buggy.  investigating, will post back here.

Comment 6

[Rick Reitmaier]

It was added since structure padding was occurring on 64b targets for the LirImm32Ins.   We need a portable way of laying out these fields, relying on the pragmas is ugly and so it ptr math and casting, but the latter might reduce churn.

Comment 7

[Steven Johnson]

Comment on attachment 367256 [details] [diff] [review]
Patch for allowing Tamarin sources to compile for Android

pushed to redux as changeset:   1629:e22e8624256f

Comment 8

[Chris Peyer]

Resolved fixed engineering / work item that has been pushed.  Setting status to verified.