Skip to content

Fix building on armv7 #155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 27, 2016
Merged

Fix building on armv7 #155

merged 1 commit into from
Aug 27, 2016

Conversation

hpux735
Copy link
Contributor

@hpux735 hpux735 commented Aug 19, 2016

This patch enables compilation from build-script on armv7. I don't expect this patch to be without controversy; rather I'd like this to be the opening point for some discussion about the specific implementation.

In particular, I'd like to draw attention to this PR at foundation: https://github.com/apple/swift-corelibs-foundation/pull/399/files which addresses val_list in the same way as presented here.

Furthermore, PAGE_SIZE is, for some reason, undefined on ARM. The definition provided here is an assumption, and kind of a hack. I'd happy to receive some guidance whether there is a better way to move forward.

Thanks!

@@ -24,6 +24,10 @@
#define DISPATCH_IO_DEBUG DISPATCH_DEBUG
#endif

#ifndef PAGE_SIZE
#define PAGE_SIZE 4096
Copy link
Contributor

@MadCoder MadCoder Aug 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PAGE_SIZE is in <sys/user.h> on linux I think.

Copy link
Contributor

@MadCoder MadCoder Aug 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup just checked, it's where it is, don't ask me how I can ever remember things like that...
it should be added to internal.h explicitly, for all platforms this is fine, BSDs have it too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very interesting. PAGE_SIZE is not defined in sys/user.h on arm. It's referenced in a few places, but I can't see how it's defined in those cases.

 wdillon@tegra-ubuntu  /usr/include  ack-grep "PAGE_SIZE"
scsi/sg.h
188:#define SG_SCATTER_SZ (8 * 4096)  /* PAGE_SIZE not available to user */
190:   The value must be a power of 2 and <= (PAGE_SIZE * 32) [131072 bytes on
191:   i386]. The minimum value is PAGE_SIZE. If scatter-gather not supported
194:   PAGE_SIZE by calling getpagesize() defined in unistd.h . */

elf.h
1882:#define PF_HP_PAGE_SIZE        0x00100000

arm-linux-gnueabihf/bits/xopen_lim.h
50:   PAGE_SIZE Size of bytes of a page.

arm-linux-gnueabihf/bits/confname.h
135:#define     _SC_PAGE_SIZE           _SC_PAGESIZE

asm-generic/shmparam.h
4:#define SHMLBA PAGE_SIZE       /* attach addr a multiple of this */

linux/resource.h
71:#define MLOCK_LIMIT  ((PAGE_SIZE > 64*1024) ? PAGE_SIZE : 64*1024)

linux/vhost.h
58:#define VHOST_PAGE_SIZE 0x1000

linux/kvm.h
352:    ((PAGE_SIZE - sizeof(struct kvm_coalesced_mmio_ring)) / \
499:#define KVM_PPC_PAGE_SIZES_MAX_SZ   8
509:    struct kvm_ppc_one_page_size enc[KVM_PPC_PAGE_SIZES_MAX_SZ];
512:#define KVM_PPC_PAGE_SIZES_REAL         0x00000001
519:    struct kvm_ppc_one_seg_page_size sps[KVM_PPC_PAGE_SIZES_MAX_SZ];

linux/binfmts.h
14:#define MAX_ARG_STRLEN (PAGE_SIZE * 32)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then you should use getpagesize() or sysconf(_SC_PAGESIZE) for the define. I assume arm doesn't define it because there are both 4k and 16k devices out there and it's not tied to the ABI.

@MadCoder MadCoder merged commit 7fe8323 into swiftlang:master Aug 27, 2016
@hpux735
Copy link
Contributor Author

hpux735 commented Aug 27, 2016

Thanks, @MadCoder !

das pushed a commit that referenced this pull request Feb 21, 2017
Fix building on armv7

Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
@zwaldowski zwaldowski mentioned this pull request Aug 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants