Support Linux on z #386
Support Linux on z #386
Conversation
| @@ -165,7 +165,11 @@ struct __CFConstStr { | |||
| uint8_t _pad[4]; | |||
| } _base; | |||
| uint8_t *_ptr; | |||
| #if __LP64__ | |||
| uint64_t _length; | |||
| #else | |||
There was a problem hiding this comment.
Hm, this is always a uint32_t on other platforms, even in 64 bit.
There was a problem hiding this comment.
I have changed the code to use unsigned long instead (same as the length field in __CFString struct which is of type CFIndex)
| @@ -165,7 +165,7 @@ struct __CFConstStr { | |||
| uint8_t _pad[4]; | |||
| } _base; | |||
| uint8_t *_ptr; | |||
| uint32_t _length; | |||
| signed long _length; | |||
There was a problem hiding this comment.
I meant to change it to be the same as the length field in __CFString struct which is of type CFIndex. CFIndex is signed long. I can change it to unsigned
| @@ -165,7 +165,7 @@ struct __CFConstStr { | |||
| uint8_t _pad[4]; | |||
| } _base; | |||
| uint8_t *_ptr; | |||
| uint32_t _length; | |||
| unsigned long _length; | |||
There was a problem hiding this comment.
This must remain the same since the emission is determined by what clang itself emits (and has emitted for previously compiled libraries) this layout is paired not only to the emission of @"" but also CFSTR("")
There was a problem hiding this comment.
If it is needed for this platform to change then it should have an ifdef for gating the change only for that specific platform.
There was a problem hiding this comment.
The aliasing of a 32-bit _length field with a 64-bit CFIndex field will not work on any 64-bit big-endian system (e.g. s390x and powerpc64). So I believe the guard should be:
#if defined(__LP64__) && defined(__BIG_ENDIAN__)
|
Thank you everyone for the reviews. I appreciate the feedback and have made changes based on the suggestions. |
vivkong
force-pushed
the
s390x-init
branch
3 times, most recently
from
9e20ded to
ecb8012
Compare
| #define HALT do {__builtin_trap(); kill(getpid(), 9); __builtin_unreachable(); } while (0) | ||
| #else | ||
| #error Compiler not supported | ||
| #endif |
There was a problem hiding this comment.
BTW, it would be nice to rebase this on #372 . It collapses the duplication.
There was a problem hiding this comment.
Thanks. I've rebased on top of it.
|
Wondering if there are other comments or suggestions for this PR? Thanks. |
|
@swift-ci please test |
Uh oh!
There was an error while loading. Please reload this page.