Skip to content

Fix build error on s390x #742

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
Dec 15, 2016
Merged

Conversation

vivkong
Copy link
Contributor

@vivkong vivkong commented Dec 12, 2016

HALT is only defined on i386 and x86_64 with recent changes. This breaks the build on s390x.

#if defined(__i386__) || defined(__x86_64__)
#if defined(__GNUC__)
#define HALT do {asm __volatile__("int3"); kill(getpid(), 9); __builtin_unreachable(); } while (0)
#if defined(__i386__) || defined(__x86_64__) || defined(__s390x__)
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a few architecture checks across CF but we don't usually check for s390x there. Is neither i386 nor x86_64 defined on that platform?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i386 and x86_64 are not defined on s390x.

Perhaps instead of doing a check on s390x, we can change the code to check like this?

#if defined(__GNUC__) 
        #if defined(__i386__) || defined(__x86_64__)
          #define HALT do {asm __volatile__("int3"); kill(getpid(), 9); __builtin_unreachable(); } while (0)
        #else
          #define HALT do {__builtin_trap(); kill(getpid(), 9); __builtin_unreachable(); } while (0)
        #endif
    #elif defined(_MSC_VER)
        #define HALT do { DebugBreak(); abort(); __builtin_unreachable(); } while (0)
    #else
        #error Compiler not supported
    #endif

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok - my main question is, though: what happens on s390x in all of the other places we switch on architecture?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like there is always a fall back when the architecture is not i386/x86_64 so we can continue on s390x. Need to take a look and see if there's work to be done for s390x in those places. In the mean time, can we please fix this so our s390x build can continue? Thanks.

@parkera
Copy link
Contributor

parkera commented Dec 15, 2016

@swift-ci please test and merge

@swift-ci swift-ci merged commit e8b5603 into swiftlang:master Dec 15, 2016
@vivkong vivkong deleted the master-s390x branch March 8, 2018 13:44
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